diff --git a/backend/apps/agents/browser/browser_agent.py b/backend/apps/agents/browser/browser_agent.py index 548157be..e49a7258 100644 --- a/backend/apps/agents/browser/browser_agent.py +++ b/backend/apps/agents/browser/browser_agent.py @@ -778,7 +778,6 @@ async def run_browser_agent( except Exception: pass session.status = "completed" - agent_manager.sync_session_close(session) await ws_manager.send_to_session(session_id, "agent:status", { "session_id": session_id, "status": "completed", "session": session.model_dump(mode="json"), @@ -1908,7 +1907,6 @@ async def run_browser_agent( except Exception as e: logger.warning(f"[browser-agent {session_id}] keep_open persist failed: {e}") - agent_manager.sync_session_close(session) await ws_manager.send_to_session(session_id, "agent:status", { "session_id": session_id, "status": final_status, diff --git a/backend/apps/agents/manager/AgentManagerProtocol.py b/backend/apps/agents/manager/AgentManagerProtocol.py index 5191ce5d..c2498883 100644 --- a/backend/apps/agents/manager/AgentManagerProtocol.py +++ b/backend/apps/agents/manager/AgentManagerProtocol.py @@ -34,7 +34,6 @@ class AgentManagerProtocol: def commit_partial_now(self, *args: Any, **kwargs: Any) -> Any: ... def stop_agent(self, *args: Any, **kwargs: Any) -> Any: ... def drain_task(self, *args: Any, **kwargs: Any) -> Any: ... - def sync_session_close(self, *args: Any, **kwargs: Any) -> Any: ... def build_mcp_servers(self, *args: Any, **kwargs: Any) -> Any: ... def build_search_text(self, *args: Any, **kwargs: Any) -> Any: ... def stream_text(self, *args: Any, **kwargs: Any) -> Any: ... diff --git a/backend/apps/agents/manager/session/SessionLifecycle.py b/backend/apps/agents/manager/session/SessionLifecycle.py index 34327f8e..0cc905a3 100644 --- a/backend/apps/agents/manager/session/SessionLifecycle.py +++ b/backend/apps/agents/manager/session/SessionLifecycle.py @@ -17,7 +17,6 @@ from backend.apps.agents.manager.session.session_store import ( save_session, build_search_text, ) -from backend.apps.agents.manager.session.sync_session_close import sync_session_close from backend.apps.agents.manager.session.apply_context_window import apply_context_window from backend.apps.agents.manager.session import resume_and_duplicate from backend.apps.agents.manager.view_builder_state import ( @@ -37,10 +36,6 @@ class SessionLifecycle(AgentManagerProtocol): def build_search_text(session: AgentSession, max_len: int = 5000) -> str: return build_search_text(session, max_len) - @typechecked - def sync_session_close(self, session: AgentSession, close_reason: str = "user"): - sync_session_close(session, close_reason) - @typechecked async def close_session(self, session_id: str) -> None: """Close a session: pause the agent if running, persist to JSON file, @@ -76,8 +71,6 @@ class SessionLifecycle(AgentManagerProtocol): if ev: ev.set() - self.sync_session_close(session) - doc_data = session.model_dump(mode="json") doc_data["search_text"] = self.build_search_text(session) diff --git a/backend/apps/agents/manager/session/SessionPersistence.py b/backend/apps/agents/manager/session/SessionPersistence.py index 7d44baac..82f5b3cb 100644 --- a/backend/apps/agents/manager/session/SessionPersistence.py +++ b/backend/apps/agents/manager/session/SessionPersistence.py @@ -1,7 +1,7 @@ """Bulk session persistence across the WHOLE store, the startup/shutdown orchestration that operates on every session at once (reconcile stale-running, flush-all on shutdown, restore-all on boot). Split from SessionLifecycle (which handles ONE session at a time) so each file is -one concern. self.sessions / self.sync_session_close resolve across the MRO as before.""" +one concern. self.sessions resolves across the MRO as before.""" import logging @@ -52,8 +52,6 @@ class SessionPersistence(AgentManagerProtocol): for req in list(session.pending_approvals): ws_manager.resolve_approval(req.id, {"behavior": "deny", "message": "Server shutting down"}) session.pending_approvals = [] - # Tag this close as "shutdown" so the cloud can tell it apart from a user-initiated close. The desktop doesn't care; the tag rides along in the dump for whoever consumes it. - self.sync_session_close(session, close_reason="shutdown") doc_data = session.model_dump(mode="json") doc_data["search_text"] = self.build_search_text(session) save_session(session_id, doc_data) diff --git a/backend/apps/agents/manager/session/sync_session_close.py b/backend/apps/agents/manager/session/sync_session_close.py deleted file mode 100644 index fa02e716..00000000 --- a/backend/apps/agents/manager/session/sync_session_close.py +++ /dev/null @@ -1,26 +0,0 @@ -"""Submit a session snapshot to the cloud on close. The cloud consumes the dump however it sees -fit; the desktop just hands off a snapshot. Skipped for mock sessions so dev runs don't post to -the real backend. Synthesizes a closed_at timestamp on the cloud-bound dump if the session lacks -one (two paths, browser_agent close and shutdown_all_sessions, previously sent it null, which -left the cloud unable to compute duration_ms). Fixed here at the bottleneck so no call site can -miss it; the on-disk session JSON keeps its original (possibly None) closed_at.""" - -from datetime import datetime - -from typeguard import typechecked - -from backend.apps.agents.core.models import AgentSession -from backend.apps.service.client import sync as submit_to_cloud - - -@typechecked -def sync_session_close(session: AgentSession, close_reason: str = "user") -> None: - if close_reason == "mock" or getattr(session, "_mock_run", False): - return - try: - dump = session.model_dump(mode="json") - if not dump.get("closed_at"): - dump["closed_at"] = datetime.now().isoformat() - submit_to_cloud(dump) - except Exception: - pass diff --git a/backend/tests/test_browser_agent_loop.py b/backend/tests/test_browser_agent_loop.py index 9aab43bd..7de09eb2 100644 --- a/backend/tests/test_browser_agent_loop.py +++ b/backend/tests/test_browser_agent_loop.py @@ -83,7 +83,6 @@ def p_install(monkeypatch, primary, aux): monkeypatch.setattr(cred_mod, "get_anthropic_client_for_model", p_client_for, raising=True) monkeypatch.setattr(BA, "load_builtin_permissions", lambda: {}, raising=True) - monkeypatch.setattr(am_mod.agent_manager, "sync_session_close", lambda *a, **k: None, raising=True) # fake WS: record browser commands, script results by action sent = [] diff --git a/backend/tests/test_service_legacy.py b/backend/tests/test_service_legacy.py index 6a7da521..a37c14d2 100644 --- a/backend/tests/test_service_legacy.py +++ b/backend/tests/test_service_legacy.py @@ -13,8 +13,6 @@ Run with: import json import os import tempfile -from unittest.mock import AsyncMock, MagicMock, patch -from uuid import uuid4 import pytest @@ -135,14 +133,6 @@ def last_sync(kind: str) -> dict: # Import application modules (after fixtures are wired). from backend.apps.service.client import record -from backend.apps.agents.core.models import AgentConfig, AgentSession, Message, ApprovalRequest -from backend.apps.agents.agent_manager import AgentManager - - -@pytest.fixture -def manager(): - """Fresh AgentManager per test.""" - return AgentManager() # --------------------------------------------------------------------------- 1. record(), legacy shim correctness --------------------------------------------------------------------------- @@ -171,45 +161,5 @@ class TestRecordBasics: assert s["properties"]["dashboard_id"] == "dash456" -# --------------------------------------------------------------------------- 2. Multi-message session, close fires exactly once --------------------------------------------------------------------------- - -class TestMultiMessageSession: - @pytest.mark.asyncio - async def test_session_completes_only_on_close(self, manager): - """Verify a completed-session sync does NOT fire mid-loop. It should - only fire on close_session() or persist_all_sessions().""" - config = AgentConfig(name="Multi-msg", model="sonnet", mode="agent") - session = await manager.launch_agent(config) - - for i in range(3): - session.messages.append(Message(role="user", content=f"msg {i}")) - session.messages.append(Message(role="assistant", content=f"reply {i}")) - - completed = syncs("session.completed") - assert len(completed) == 0, f"session-completed fired {len(completed)} times before close" - - session.status = "completed" - await manager.close_session(session.id) - - completed = syncs("session.completed") - assert len(completed) == 1, f"expected 1 completed sync, got {len(completed)}" - - -# --------------------------------------------------------------------------- 3. Token + cost capture on close --------------------------------------------------------------------------- - -class TestTokenTracking: - @pytest.mark.asyncio - async def test_tokens_and_cost_in_session_close(self, manager): - config = AgentConfig(name="Token Test", model="opus", mode="agent") - session = await manager.launch_agent(config) - - session.tokens = {"input": 50000, "output": 15000} - session.cost_usd = 0.25 - session.status = "completed" - - await manager.close_session(session.id) - - s = last_sync("session.completed") - assert s["properties"]["tokens"]["input"] == 50000 - assert s["properties"]["tokens"]["output"] == 15000 - assert s["properties"]["cost_usd"] == 0.25 +# Session-close cloud sync (the old session.completed snapshot path) was retired in favour of Haik's +# per-message product-analytics bridge, so its close-fires-once + token-on-close tests were removed.