From 616d4ffb67510d19786e835c9ffb6201959cd702 Mon Sep 17 00:00:00 2001 From: "hamza.khyari" Date: Mon, 18 May 2026 12:40:50 +0000 Subject: [PATCH] fix(tui): bind active persona actor at session creation and dispatch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per spec §TUI Session Lifecycle: the active persona actor must be bound to the session at startup, and passed as an override on every dispatch so persona cycling (tab) takes effect immediately. - _create_tui_session(persona_state) — passes active persona actor to service.create() so first-launch message does not hit SessionActorNotConfiguredError - _run_llm_dispatch(... actor_override) — accepts and forwards actor override in A2aRequest params so persona switching works without restarting the TUI - on_input_submitted — resolves active_persona(session_id).actor and passes it as actor_override on each dispatch - 2 new BDD scenarios: actor_override forwarding, persona-bound session creation (8 scenarios total, all passing) Refs: #11230 --- features/steps/tui_llm_dispatch_steps.py | 52 +++++++++++++++++++++++- features/tui_llm_dispatch.feature | 10 +++++ src/cleveragents/tui/app.py | 37 +++++++++++++---- src/cleveragents/tui/commands.py | 15 +++++-- 4 files changed, 102 insertions(+), 12 deletions(-) diff --git a/features/steps/tui_llm_dispatch_steps.py b/features/steps/tui_llm_dispatch_steps.py index cedfe7470..ee9f3b815 100644 --- a/features/steps/tui_llm_dispatch_steps.py +++ b/features/steps/tui_llm_dispatch_steps.py @@ -118,9 +118,27 @@ def step_submit_text(context: Any, text: str) -> None: ) +@when('I dispatch "{text}" overriding actor with "{actor}"') +def step_dispatch_text_with_actor(context: Any, text: str, actor: str) -> None: + """Invoke _run_llm_dispatch with an explicit actor_override.""" + context._result_text = _run_llm_dispatch( + context._facade, context._session_id, text, actor_override=actor + ) + + +@given('a persona state with active actor "{actor}"') +def step_persona_state_with_actor(context: Any, actor: str) -> None: + """Set up a mock PersonaState whose active persona has the given actor.""" + persona = MagicMock() + persona.actor = actor + state = MagicMock() + state.active_persona.return_value = persona + context._mock_persona_state = state + + @when("I call _create_tui_session with that service") def step_call_create_tui_session(context: Any) -> None: - """Call _create_tui_session patching get_container.""" + """Call _create_tui_session patching get_container, no persona state.""" from cleveragents.tui import commands as cmd_mod container = MagicMock() @@ -129,6 +147,19 @@ def step_call_create_tui_session(context: Any) -> None: context._result_sid = cmd_mod._create_tui_session() +@when("I call _create_tui_session with that service and persona state") +def step_call_create_tui_session_with_persona(context: Any) -> None: + """Call _create_tui_session with a persona state so actor is bound.""" + from cleveragents.tui import commands as cmd_mod + + container = MagicMock() + container.session_service.return_value = context._mock_service + with patch.object(cmd_mod, "get_container", return_value=container): + context._result_sid = cmd_mod._create_tui_session( + persona_state=context._mock_persona_state + ) + + # --------------------------------------------------------------------------- # Then steps # --------------------------------------------------------------------------- @@ -184,3 +215,22 @@ def step_returned_session_id(context: Any, expected: str) -> None: assert context._result_sid == expected, ( f"Expected session_id={expected!r}, got {context._result_sid!r}" ) + + +@then('the facade should have received a message/send request with actor "{actor}"') +def step_facade_received_request_with_actor(context: Any, actor: str) -> None: + """Assert the dispatch request included the given actor override.""" + context._facade.dispatch.assert_called_once() + request = context._facade.dispatch.call_args[0][0] + assert request.method == "message/send", ( + f"Expected method='message/send', got {request.method!r}" + ) + assert request.params.get("actor") == actor, ( + f"Expected actor={actor!r} in params, got {request.params.get('actor')!r}" + ) + + +@then('the session should have been created with actor "{actor}"') +def step_session_created_with_actor(context: Any, actor: str) -> None: + """Assert service.create() was called with the persona's actor.""" + context._mock_service.create.assert_called_once_with(actor_name=actor) diff --git a/features/tui_llm_dispatch.feature b/features/tui_llm_dispatch.feature index 718ce6b03..8b86f0b34 100644 --- a/features/tui_llm_dispatch.feature +++ b/features/tui_llm_dispatch.feature @@ -11,6 +11,10 @@ Feature: TUI normal text input dispatches to LLM via A2A facade Then the facade should have received a message/send request And the conversation should show the assistant response + Scenario: Dispatch passes active persona actor as override + When I dispatch "hello there" overriding actor with "openai/gpt-4o" + Then the facade should have received a message/send request with actor "openai/gpt-4o" + Scenario: Normal text with no facade falls back to preview mode Given the TUI app is initialised without a facade When I submit the text "hello there" @@ -35,3 +39,9 @@ Feature: TUI normal text input dispatches to LLM via A2A facade Given the session service raises an exception When I call _create_tui_session with that service Then the returned session_id should be "default" + + Scenario: _create_tui_session binds active persona actor at session creation + Given a mock session service that returns session_id "abc-123" + And a persona state with active actor "anthropic/claude-4-sonnet" + When I call _create_tui_session with that service and persona state + Then the session should have been created with actor "anthropic/claude-4-sonnet" diff --git a/src/cleveragents/tui/app.py b/src/cleveragents/tui/app.py index a2fd5654e..c7177e7c8 100644 --- a/src/cleveragents/tui/app.py +++ b/src/cleveragents/tui/app.py @@ -30,12 +30,31 @@ else: InputSubmittedEvent = Any -def _run_llm_dispatch(facade: A2aLocalFacade, session_id: str, message: str) -> str: +def _run_llm_dispatch( + facade: A2aLocalFacade, + session_id: str, + message: str, + actor_override: str | None = None, +) -> str: """Dispatch *message* to the LLM via the A2A facade and return display text. This is a pure module-level function (no Textual dependency) so it can be unit-tested directly without a running event loop or mocked Textual. + Parameters + ---------- + facade: + The wired A2A local facade. + session_id: + Database-backed session ULID. + message: + User message text (with ``@`` references already expanded). + actor_override: + When set, overrides the session's default actor for this request. + Used to pass the active persona's actor on every dispatch so that + persona cycling (``tab`` / ``action_cycle_preset``) is reflected + immediately without requiring a session restart. + Returns a formatted string ready for the conversation widget: - On success: ``"You: \\n\\nAssistant: "`` - On no-actor: friendly guidance message @@ -44,13 +63,12 @@ def _run_llm_dispatch(facade: A2aLocalFacade, session_id: str, message: str) -> from cleveragents.a2a.models import A2aRequest from cleveragents.domain.models.core.session import SessionActorNotConfiguredError + params: dict[str, Any] = {"session_id": session_id, "message": message} + if actor_override: + params["actor"] = actor_override + try: - response = facade.dispatch( - A2aRequest( - method="message/send", - params={"session_id": session_id, "message": message}, - ) - ) + response = facade.dispatch(A2aRequest(method="message/send", params=params)) if response.error is not None: return f"[Error] {response.error.message}" result_data: dict[str, Any] = response.result or {} @@ -260,11 +278,14 @@ if _TEXTUAL_AVAILABLE: # Run blocking LLM call off the main Textual thread so the # event loop stays responsive during the API round-trip. + # Pass the active persona's actor as an override on every + # request so persona cycling (tab) takes effect immediately. facade = self._facade session_id = self._session.session_id + actor = self._persona_state.active_persona(session_id).actor or None def _dispatch_llm() -> str: - return _run_llm_dispatch(facade, session_id, expanded) + return _run_llm_dispatch(facade, session_id, expanded, actor) def _on_llm_done(worker: Any) -> None: """Update conversation widget once worker finishes.""" diff --git a/src/cleveragents/tui/commands.py b/src/cleveragents/tui/commands.py index c60b0f747..7c9a064b6 100644 --- a/src/cleveragents/tui/commands.py +++ b/src/cleveragents/tui/commands.py @@ -271,18 +271,27 @@ def _build_tui_facade() -> A2aLocalFacade | None: return None -def _create_tui_session() -> str: +def _create_tui_session(persona_state: PersonaState | None = None) -> str: """Create a real database-backed TUI session and return its session_id. + When *persona_state* is provided, the active persona's actor is bound to + the session at creation time so the first message dispatches to the LLM + without hitting ``SessionActorNotConfiguredError``. + Falls back to the string ``"default"`` if the session service is unavailable so the TUI can still launch without a database. """ import contextlib + actor_name: str | None = None + if persona_state is not None: + with contextlib.suppress(Exception): + actor_name = persona_state.active_persona("default").actor or None + with contextlib.suppress(Exception): container = get_container() service = container.session_service() - session = service.create(actor_name=None) + session = service.create(actor_name=actor_name) return session.session_id return "default" # graceful fallback @@ -307,7 +316,7 @@ def run_tui(*, headless: bool = False) -> int: return 0 facade = _build_tui_facade() - session_id = _create_tui_session() + session_id = _create_tui_session(persona_state=state) app = CleverAgentsTuiApp( command_router=router,