diff --git a/backend/apps/agents/manager/SessionControl.py b/backend/apps/agents/manager/SessionControl.py index 497442ed..b74ee0f5 100644 --- a/backend/apps/agents/manager/SessionControl.py +++ b/backend/apps/agents/manager/SessionControl.py @@ -41,15 +41,18 @@ class SessionControl(AgentManagerProtocol): ws_manager.resolve_approval(req.id, {"behavior": "deny", "message": "Agent stopped"}) session.pending_approvals = [] - session.status = "stopped" - session.needs_fresh_session = True - if not session.closed_at: - session.closed_at = datetime.now() - # Persist the partial reply NOW, before tearing down the SDK. The cancel handler also does this, but it sits behind the generator's teardown, which can take several seconds; doing it here means the streamed text stays put the instant Stop is pressed instead of blinking out and reappearing once teardown finishes. - await self.commit_partial_now(session) + # Only a LIVE turn can be stopped. A finished session's task lingers in the registry, and shutdown stops every task it finds, so an unconditional flip relabelled every completed chat "stopped" on restart and hung a Resume button off a conversation that was already answered. + p_was_live = session.status in ("running", "waiting_approval") + if p_was_live: + session.status = "stopped" + session.needs_fresh_session = True + if not session.closed_at: + session.closed_at = datetime.now() + # Persist the partial reply NOW, before tearing down the SDK. The cancel handler also does this, but it sits behind the generator's teardown, which can take several seconds; doing it here means the streamed text stays put the instant Stop is pressed instead of blinking out and reappearing once teardown finishes. + await self.commit_partial_now(session) await ws_manager.send_to_session(session_id, "agent:status", { "session_id": session_id, - "status": "stopped", + "status": session.status, "session": session.model_dump(mode="json"), }) # Snapshot now: the cancelled task's finally skips the save (it's no longer the live task once we pop it below), so persist the partial here or it'd live only in memory until the next turn / shutdown. diff --git a/backend/tests/test_stop_agent_preserves_terminal_status.py b/backend/tests/test_stop_agent_preserves_terminal_status.py new file mode 100644 index 00000000..5803f176 --- /dev/null +++ b/backend/tests/test_stop_agent_preserves_terminal_status.py @@ -0,0 +1,60 @@ +"""Stopping is for LIVE turns. A settled chat must come back settled. + +Found live: quit the app and relaunch, and every finished conversation came back labelled +"stopped" with a Resume button hanging off an answer that was already delivered. The chain is +short. `self.tasks[session_id]` is written when a turn starts and only removed by stop/close, so a +completed session leaves its done task in the registry forever; the agents sub-app's shutdown hook +loops over every task it finds and calls `stop_agent` on each; `stop_agent` then wrote +`status = "stopped"` unconditionally. One ordinary session was enough to reproduce it. + +The guard belongs in `stop_agent`, not in the shutdown loop, so no future caller can express the +bad state either. `close_session` already had exactly this guard, which is what made the +inconsistency easy to miss. + +Run: + cd backend && .venv/bin/python -m pytest tests/test_stop_agent_preserves_terminal_status.py -v +""" + +from __future__ import annotations + +import pytest + +from backend.apps.agents.agent_manager import agent_manager +from backend.apps.agents.core.models import AgentSession, Message + + +def p_seed(status: str) -> AgentSession: + s = AgentSession(name="t", model="sonnet") + s.status = status + s.messages = [Message(role="user", content="hi"), Message(role="assistant", content="hello")] + agent_manager.sessions[s.id] = s + return s + + +@pytest.mark.asyncio +@pytest.mark.parametrize("terminal", ["completed", "error", "stopped"]) +async def test_stopping_a_settled_chat_leaves_its_status_alone(terminal: str) -> None: + s = p_seed(terminal) + try: + await agent_manager.stop_agent(s.id) + assert s.status == terminal, "a restart must not relabel a finished conversation" + assert s.needs_fresh_session is False, "nothing was interrupted, so nothing needs rebuilding" + assert s.closed_at is None + finally: + agent_manager.sessions.pop(s.id, None) + agent_manager.tasks.pop(s.id, None) + + +@pytest.mark.asyncio +@pytest.mark.parametrize("live", ["running", "waiting_approval"]) +async def test_stopping_a_live_turn_still_stops_it(live: str) -> None: + """The discriminating half: the Stop button must keep working.""" + s = p_seed(live) + try: + await agent_manager.stop_agent(s.id) + assert s.status == "stopped" + assert s.needs_fresh_session is True + assert s.closed_at is not None + finally: + agent_manager.sessions.pop(s.id, None) + agent_manager.tasks.pop(s.id, None)