diff --git a/backend/apps/agents/agent_manager.py b/backend/apps/agents/agent_manager.py index 2f10b257..3c2654db 100644 --- a/backend/apps/agents/agent_manager.py +++ b/backend/apps/agents/agent_manager.py @@ -50,7 +50,13 @@ class AgentManager(SessionLifecycle, SessionPersistence, Messaging, SessionContr already resumes the work, so the stale continuation quietly stands down.""" if delay_s > 0: p_before = len(getattr(self.sessions.get(session_id), "messages", []) or []) - await asyncio.sleep(delay_s) + p_parked = self.sessions.get(session_id) + if p_parked is not None and getattr(p_parked, "awaiting_reconnect", False): + # An outage wait is a CEILING, not a sentence: a blind sleep strands the user long after their wifi returned. Rotation waits keep the flat sleep, where the window IS the point. + from backend.apps.agents.manager.run.reconnect_resume import wait_for_reconnect + await wait_for_reconnect(p_parked, delay_s) + else: + await asyncio.sleep(delay_s) p_session = self.sessions.get(session_id) if p_session is None: return @@ -229,6 +235,9 @@ class AgentManager(SessionLifecycle, SessionPersistence, Messaging, SessionContr force_respawn=p_force_respawn, ) session.status = "completed" + # The turn got through, so the outage is over: the next unrelated blip starts from a full budget. + from backend.apps.agents.manager.run.reconnect_resume import clear_reconnect_wait + clear_reconnect_wait(session) # Silent-quit seal: a turn that ran tools and ended with no visible answer gets ONE hidden continue nudge (dispatched by the auto-continuation block below); a second silent quit in the same ask surfaces as-is rather than looping. try: diff --git a/backend/apps/agents/core/error_classify.py b/backend/apps/agents/core/error_classify.py index 8b78a394..0d05482d 100644 --- a/backend/apps/agents/core/error_classify.py +++ b/backend/apps/agents/core/error_classify.py @@ -328,6 +328,17 @@ def is_cert_failure(exc: BaseException, extra_text: str = "") -> bool: return bool(CERT_FAILURE_PATTERNS.search(f"{exc!s}\n{extra_text}")) +@typechecked +def is_connection_lost(exc: BaseException) -> bool: + """True when the transport itself died, as opposed to the provider answering with a refusal. + + Both arrive as "transient", but they want different recoveries: a dead socket leaves the CLI + holding a corpse and must respawn, while a 429 is a healthy connection carrying a NO, where + respawning just spends a process to be told the same thing. + """ + return isinstance(exc, p_get_transient_exc_types()) + + @typechecked def is_transient_capacity_error(exc: BaseException, extra_text: str = "") -> bool: # The Claude CLI's underlying ProcessError stringifies to a generic "Command failed with exit code 1 / Check stderr output for details"; the real cause (rate_limit_error / No pool capacity available / 429 / overloaded) only surfaces in the subprocess's stderr stream, which we capture via the SDK's `stderr` callback and pass in as extra_text. Classify against both so we catch capacity errors regardless of which channel carried the message. diff --git a/backend/apps/agents/core/models.py b/backend/apps/agents/core/models.py index 4a574540..0ad2ba6d 100644 --- a/backend/apps/agents/core/models.py +++ b/backend/apps/agents/core/models.py @@ -152,6 +152,10 @@ class AgentSession(BaseModel): suppress_recap_once: bool = False # Consecutive dirty deaths this session was MID-TURN for; the crash auto-resume breaker (hermes #30719 pairing: auto-resume must never outrun its circuit breaker). crash_interrupt_count: int = 0 + # Outage rounds spent on this ask: the in-turn ladder covers only 335s, and the work is checkpointed, so a longer drop is waited out rather than ending the task. + reconnect_attempts: int = 0 + # True while a turn is parked waiting for the connection back; persisted so a quit DURING the wait is still an owed turn at next boot. + awaiting_reconnect: bool = False # Seconds the auto-continuation dispatcher sleeps before sending (codex rotation windows last 1-2 min; an instant retry lands inside the same window and burns the one-shot budget). pending_continuation_delay_s: int = 0 # Memory prompt block frozen at first compose (prefix-cache discipline: mid-chat fact writes must diff --git a/backend/apps/agents/manager/Messaging.py b/backend/apps/agents/manager/Messaging.py index bc1e1cdf..30b60e7d 100644 --- a/backend/apps/agents/manager/Messaging.py +++ b/backend/apps/agents/manager/Messaging.py @@ -152,6 +152,9 @@ class Messaging(AgentManagerProtocol): session.empty_finish_progress_mark = 0 session.empty_finish_surfaced = False session.auth_retry_used = False + # A human is here and driving, so an earlier outage stops counting against the next one. + session.reconnect_attempts = 0 + session.awaiting_reconnect = False # Fire a background aux LLM call to generate a 3-6 word verb-phrase describing this turn ("Auditing the pull request", "Drafting your email"). The narrator pill swaps from its heuristic verb to this label as soon as it lands, usually ~500ms-1s into the turn, which is exactly when "Thinking…" starts feeling generic. Provider-agnostic via resolve_aux_model. Non-blocking; failure is silent and the heuristic stays. if not hidden and prompt: try: diff --git a/backend/apps/agents/manager/run/handle_run_error.py b/backend/apps/agents/manager/run/handle_run_error.py index b4a1f156..852c128e 100644 --- a/backend/apps/agents/manager/run/handle_run_error.py +++ b/backend/apps/agents/manager/run/handle_run_error.py @@ -21,6 +21,7 @@ from backend.apps.agents.core.error_classify import ( is_auth_error, is_cert_failure, is_cli_binary_missing, + is_connection_lost, is_unknown_model_error, parse_retry_after, ) @@ -178,6 +179,17 @@ async def handle_run_error(e: Exception, session: AgentSession, session_id: str, logger.debug("submit_diagnostic cli_binary_missing failed", exc_info=True) elif is_transient_capacity_error(e, extra_text=p_stderr_tail): # A genuine throttle (429/overload/capacity) that already burned the whole silent-backoff budget (the only way one reaches here). It's a limit, not a failure, so don't append a system-message card; emit a transient signal for the muted pill and mark the turn completed so it doesn't read as an error. + # 335s of ladder is a blip's worth of patience, and a closed lid or switched network outlasts it, so park and retry before conceding a turn the user never chose to end. + from backend.apps.agents.manager.run.reconnect_resume import arm_reconnect_resume + p_delay = arm_reconnect_resume(session, parse_retry_after(e, p_stderr_tail), is_connection_lost(e)) + if p_delay is not None: + logger.info(f"Agent {session_id}: connection lost past the in-turn budget; retrying in {p_delay}s") + await ws_manager.send_to_session(session_id, "agent:reconnect_wait", { + "session_id": session_id, + "retry_in_s": p_delay, + "attempt": session.reconnect_attempts, + }) + return session.status = "completed" if turn.stream_text_msg_id: try: diff --git a/backend/apps/agents/manager/run/reconnect_resume.py b/backend/apps/agents/manager/run/reconnect_resume.py new file mode 100644 index 00000000..35e8fdf6 --- /dev/null +++ b/backend/apps/agents/manager/run/reconnect_resume.py @@ -0,0 +1,136 @@ +"""Keep a turn alive across an outage that outlasts the turn's own retry budget. + +The in-turn ladder (CAPACITY_BACKOFFS) spends 335s and then gives up, which is the right call for +a blip. It is the wrong call for a closed lid, a switched network, a hotel captive portal or a +provider having a bad ten minutes: the user comes back to a task that stopped for a reason that was +never theirs, and has to retype it. + +Nothing about that is unrecoverable. The transcript is already checkpointed, the tools that ran are +still recorded, and the continuation seam that the auth self-heal uses will pick the work back up +mid-task. So an outage becomes a wait on a widening schedule rather than an ending. + +Two properties this deliberately keeps: + - it is bounded (three rounds, then the honest pill), because a retry loop with no end is how you + burn a user's quota on a provider that is genuinely gone; + - the wait is PERSISTED, so quitting mid-wait leaves an owed turn that boot-restore resumes, + instead of a task that evaporated while nobody was looking. +""" + +import asyncio +import logging +from typing import Optional + +from typeguard import typechecked + +from backend.apps.agents.core.models import AgentSession + +logger = logging.getLogger(__name__) + +# Widening but short at the start: most outages are seconds, and a user should see it heal itself rather than learn to press a button. +RECONNECT_BACKOFFS = (60, 300, 900) + +# A provider's own "reset after" hint outranks our schedule, but capped, because a bad hint must not park a turn for an hour. +RECONNECT_MAX_DELAY_S = 1800 + +RECONNECT_PROMPT = ( + "The connection to the model dropped and has just come back. Continue exactly where you left " + "off; do not redo completed steps." +) + + +@typechecked +def arm_reconnect_resume(session: AgentSession, retry_after_s: Optional[int] = None, + connection_lost: bool = False) -> Optional[int]: + """Park the turn and queue one more attempt. Returns the delay armed, or None when the budget + is spent and the caller should surface the honest pill instead. + + Retrying the work IS the connectivity test, so there is no separate reachability oracle to get + wrong: either the next attempt goes through, or it fails and buys the next (longer) round. + """ + if session.pending_continuation: + return None + attempts = int(getattr(session, "reconnect_attempts", 0) or 0) + if attempts >= len(RECONNECT_BACKOFFS): + return None + + delay = RECONNECT_BACKOFFS[attempts] + if retry_after_s and retry_after_s > 0: + delay = max(delay, min(int(retry_after_s) + 5, RECONNECT_MAX_DELAY_S)) + + session.reconnect_attempts = attempts + 1 + session.awaiting_reconnect = True + # Only a dead transport leaves the CLI holding a corpse; a 429 is a healthy pipe carrying a NO, and respawning for that spends a process to be told the same thing. + if connection_lost: + session.needs_fresh_session = True + session.pending_continuation = True + session.pending_continuation_prompt = RECONNECT_PROMPT + session.pending_continuation_delay_s = delay + return delay + + +@typechecked +def clear_reconnect_wait(session: AgentSession) -> None: + """A turn that got through ends the outage: drop the parked flag so a later, unrelated blip + starts from a full budget rather than inheriting this one's.""" + session.awaiting_reconnect = False + + +# How often to look while parked: short enough that a wifi blip costs seconds, long enough to stay cheap over a 15 minute outage. +RECONNECT_POLL_S = 3 + +# The probe must be fast: a hung connect would turn "check every 3s" into "check whenever the socket gives up". +RECONNECT_PROBE_TIMEOUT_S = 1.5 + + +@typechecked +def provider_probe_host(session: AgentSession) -> str: + """The host whose reachability actually decides whether a retry can succeed. + + Router-backed lanes point the CLI at localhost, so probing THAT would come back healthy while + the machine is offline, which is the wrong answer at the only moment it matters. Probe the + provider the router is proxying to instead. + """ + p_model = (getattr(session, "model", "") or "").lower() + if p_model.startswith(("cx/", "gpt-")) or "openai" in p_model: + return "api.openai.com" + if p_model.startswith(("gc/", "ag/", "gemini")) or "gemini" in p_model: + return "generativelanguage.googleapis.com" + return "api.anthropic.com" + + +@typechecked +async def provider_reachable(host: str) -> bool: + """True when a TCP connection to the provider completes. Deliberately not an HTTP request: no + auth, no cost, no quota, and nothing that a retry would have spent anyway.""" + try: + p_fut = asyncio.open_connection(host, 443) + reader, writer = await asyncio.wait_for(p_fut, timeout=RECONNECT_PROBE_TIMEOUT_S) + writer.close() + try: + await writer.wait_closed() + except Exception: + pass + return True + except Exception: + return False + + +@typechecked +async def wait_for_reconnect(session: AgentSession, ceiling_s: int) -> None: + """Sleep until the provider answers again, or until the ceiling, whichever comes FIRST. + + The backoff is a bound on patience, never a fixed sentence: a blind sleep would leave a user + watching a spinner for fourteen more minutes after their wifi already came back, which is worse + than the button it replaced. A captive portal can still answer TCP and fail the real request; + that costs one round and lands us exactly where a blind wait would have been anyway. + """ + p_host = provider_probe_host(session) + p_waited = 0 + while p_waited < ceiling_s: + p_step = min(RECONNECT_POLL_S, ceiling_s - p_waited) + await asyncio.sleep(p_step) + p_waited += p_step + if await provider_reachable(p_host): + logger.info(f"reconnect: {p_host} answered after {p_waited}s (ceiling was {ceiling_s}s)") + return + logger.info(f"reconnect: ceiling {ceiling_s}s reached without {p_host} answering; trying anyway") diff --git a/backend/apps/agents/manager/session/SessionPersistence.py b/backend/apps/agents/manager/session/SessionPersistence.py index dff427e1..bbf4ff40 100644 --- a/backend/apps/agents/manager/session/SessionPersistence.py +++ b/backend/apps/agents/manager/session/SessionPersistence.py @@ -49,6 +49,19 @@ class SessionPersistence(AgentManagerProtocol): self.crash_resume_queue.append(sid) else: logger.warning(f"crash-resume breaker: session {sid} was mid-turn at {count} consecutive dirty deaths; leaving it for the manual chip") + elif data.get("awaiting_reconnect") and data.get("closed_at") is None: + # Parked mid-outage when the app went down. The file says "completed" only because + # the wait was dispatched as a continuation, so the status check above cannot see + # it; without this the task the user never chose to end just evaporates. + data["awaiting_reconnect"] = False + dirty = True + count = int(data.get("crash_interrupt_count", 0) or 0) + 1 + data["crash_interrupt_count"] = count + if count <= 1: + self.crash_resume_queue.append(sid) + else: + logger.warning(f"crash-resume breaker: session {sid} was parked mid-outage at {count} consecutive dirty deaths; leaving it for the manual chip") + # Mode migration: Chat was merged into Ask. Rewrite mode="chat" so old sessions keep loading after the chat.json file is gone. if data.get("mode") == "chat": data["mode"] = "ask" diff --git a/backend/apps/oauth_state.py b/backend/apps/oauth_state.py index 39967f10..55d60ce2 100644 --- a/backend/apps/oauth_state.py +++ b/backend/apps/oauth_state.py @@ -1,10 +1,108 @@ -# In-memory store for pending OAuth flows (state -> {provider, code_verifier, redirect_uri}) -pending_oauth: dict[str, dict] = {} +"""Pending and recently-completed OAuth flows. + +The pending map used to live only in memory, and the gap that opens is the whole of ENG-363: the +user clicks Connect, the browser leaves for the provider, the backend restarts for ANY reason +(uvicorn reload in dev, the ENG-357 frozen-loop exit, a watchdog respawn, a crash), and the state +that proves the returning callback is ours is simply gone. The callback then lands on the +unknown-state branch and renders "Session expired", the Settings row spins forever, and the user +concludes the product cannot connect to Anthropic. Haik reported exactly that. + +Retrying is not the fix, because the user is not the one who failed. Making the state outlive a +restart is: the callback then completes on its own and there is nothing to click. + +The verifier is a short-lived, single-use secret, so it is written 0600, expires on a TTL, and is +deleted the moment it is consumed. It never becomes a durable credential lying around on disk. +""" + +import json +import os +import time +from typing import Dict, Optional + +from typeguard import typechecked + +from backend.config.paths import DATA_ROOT + +# One OAuth round trip is a browser hop and a login; a quarter hour is generous for a human doing that, and short enough that an abandoned flow's verifier does not linger. +PENDING_TTL_S = 15 * 60 + +PENDING_PATH = os.path.join(DATA_ROOT, "pending_oauth.json") + + +@typechecked +def p_load() -> Dict[str, dict]: + """Read the durable map, dropping anything past its TTL. Unreadable state is treated as empty: a corrupt file must not make Connect permanently impossible.""" + try: + with open(PENDING_PATH, encoding="utf-8") as fh: + raw = json.load(fh) + except Exception: + return {} + if not isinstance(raw, dict): + return {} + now = time.time() + return { + k: v for k, v in raw.items() + if isinstance(v, dict) and float(v.get("stored_at", 0) or 0) + PENDING_TTL_S > now + } + + +@typechecked +def p_store(entries: Dict[str, dict]) -> None: + """Write 0600 and replace atomically, so a crash mid-write cannot leave a half-parsed file that strands every later Connect.""" + try: + os.makedirs(DATA_ROOT, exist_ok=True) + tmp = f"{PENDING_PATH}.tmp" + with open(tmp, "w", encoding="utf-8") as fh: + json.dump(entries, fh) + os.chmod(tmp, 0o600) + os.replace(tmp, PENDING_PATH) + except OSError: + pass + + +class PendingOAuth: + """Dict-shaped so every existing call site keeps working, but backed by disk. + + Deliberately not a plain dict subclass: the whole point is that reads come from the file, so an + entry written before a restart is still found by the process that comes back. + """ + + @typechecked + def __setitem__(self, state: str, value: dict) -> None: + entries = p_load() + entries[state] = {**value, "stored_at": time.time()} + p_store(entries) + + @typechecked + def get(self, state: str, default: Optional[dict] = None) -> Optional[dict]: + return p_load().get(state, default) + + @typechecked + def pop(self, state: str, default: Optional[dict] = None) -> Optional[dict]: + entries = p_load() + found = entries.pop(state, None) + if found is None: + return default + # Consumed: the verifier is single-use, so it stops existing here rather than aging out later. + p_store(entries) + return found + + @typechecked + def __contains__(self, state: str) -> bool: + return state in p_load() + + @typechecked + def __len__(self) -> int: + return len(p_load()) + + +pending_oauth = PendingOAuth() # Recently-completed OAuth states so the /api/subscriptions/callback handler can distinguish a legitimate duplicate callback (browser prefetch, refresh, or Google redirect retry after a slow first response) from a truly stale request. Bounded FIFO, drops the oldest entries once it grows past MAX_COMPLETED_OAUTH so it can't leak memory. completed_oauth: list[str] = [] MAX_COMPLETED_OAUTH = 64 +@typechecked def mark_oauth_completed(state: str) -> None: if state in completed_oauth: return diff --git a/backend/tests/test_oauth_state_durable.py b/backend/tests/test_oauth_state_durable.py new file mode 100644 index 00000000..470a9ce7 --- /dev/null +++ b/backend/tests/test_oauth_state_durable.py @@ -0,0 +1,68 @@ +"""ENG-363: a Connect that survives a backend restart. + +The user clicks Connect, the browser leaves for Anthropic, the backend restarts for any reason, and +the returning callback used to find nothing and render "Session expired" while Settings spun +forever. Haik reported that as "Model connection for anthropic does not work". The state is ours to +keep, so keeping it is the fix; there is nothing for the user to retry. +""" + +import json +import os +import time + +import pytest + + +@pytest.fixture() +def p_store(monkeypatch, tmp_path): + import backend.apps.oauth_state as st + monkeypatch.setattr(st, "PENDING_PATH", str(tmp_path / "pending_oauth.json"), raising=True) + monkeypatch.setattr(st, "DATA_ROOT", str(tmp_path), raising=True) + return st + + +def test_a_pending_flow_survives_a_restart(p_store): + p_store.pending_oauth["state-abc"] = { + "provider": "claude", "code_verifier": "v", "redirect_uri": "http://localhost:20128/cb", + } + # A restart is a brand new object over the same file; the old process kept nothing. + fresh = p_store.PendingOAuth() + found = fresh.pop("state-abc") + assert found is not None, "the callback must still recognise its own flow" + assert found["code_verifier"] == "v" + + +def test_consuming_a_flow_deletes_the_verifier(p_store): + p_store.pending_oauth["state-abc"] = {"provider": "claude", "code_verifier": "secret"} + p_store.pending_oauth.pop("state-abc") + on_disk = json.load(open(p_store.PENDING_PATH)) + assert on_disk == {}, "a single-use secret must not outlive its use" + + +def test_an_abandoned_flow_ages_out(p_store): + p_store.pending_oauth["stale"] = {"provider": "claude", "code_verifier": "x"} + raw = json.load(open(p_store.PENDING_PATH)) + raw["stale"]["stored_at"] = time.time() - (p_store.PENDING_TTL_S + 60) + open(p_store.PENDING_PATH, "w").write(json.dumps(raw)) + assert p_store.pending_oauth.get("stale") is None + assert "stale" not in p_store.pending_oauth + + +def test_the_verifier_is_not_world_readable(p_store): + p_store.pending_oauth["state-abc"] = {"provider": "claude", "code_verifier": "secret"} + mode = os.stat(p_store.PENDING_PATH).st_mode & 0o777 + assert mode == 0o600, f"pending verifiers must be owner-only, got {oct(mode)}" + + +def test_a_corrupt_file_never_blocks_connecting(p_store): + """Negative control: unreadable state must degrade to 'no pending flow', never to an exception + that makes Connect impossible forever.""" + open(p_store.PENDING_PATH, "w").write("{not json") + assert p_store.pending_oauth.get("anything") is None + p_store.pending_oauth["fresh"] = {"provider": "claude", "code_verifier": "v"} + assert p_store.pending_oauth.get("fresh") is not None, "a new flow still works" + + +def test_an_unknown_state_is_still_unknown(p_store): + """Negative control: durability must not make the handler accept a state nobody issued.""" + assert p_store.pending_oauth.pop("never-issued") is None diff --git a/backend/tests/test_reconnect_resume.py b/backend/tests/test_reconnect_resume.py new file mode 100644 index 00000000..7528bc8a --- /dev/null +++ b/backend/tests/test_reconnect_resume.py @@ -0,0 +1,212 @@ +"""An outage longer than the in-turn ladder must park the turn, not end it. + +The 335s of CAPACITY_BACKOFFS is a blip's worth of patience. A closed lid, a switched network or a +provider's bad ten minutes outlasts it, and the user then comes back to a task that stopped for a +reason that was never theirs. These pin the widening retry, its bound, and the persistence that +makes quitting mid-wait survivable. Cross-platform by construction: file state and asyncio only, +no signals and no platform paths. +""" + +import asyncio + +import backend.apps.agents.core.ws_manager as ws_mod +from backend.apps.agents.core.models import AgentSession, Message +from backend.apps.agents.manager.run.handle_run_error import handle_run_error +from backend.apps.agents.manager.run.reconnect_resume import ( + RECONNECT_BACKOFFS, + RECONNECT_MAX_DELAY_S, + arm_reconnect_resume, + clear_reconnect_wait, +) +from backend.apps.agents.manager.streaming.state import TurnState + + +def p_session() -> AgentSession: + s = AgentSession(name="t", model="sonnet", dashboard_id="d") + s.messages.append(Message(role="user", content="long task", branch_id=s.active_branch_id)) + return s + + +def p_drive(monkeypatch, exc, session=None, stderr=None): + events = [] + + async def fake_send(session_id, event, data): + events.append((event, data)) + + monkeypatch.setattr(ws_mod.ws_manager, "send_to_session", fake_send, raising=True) + import backend.apps.service.client as service_client + monkeypatch.setattr(service_client, "submit_diagnostic", lambda payload: None, raising=True) + session = session or p_session() + asyncio.run(handle_run_error(exc, session, session.id, TurnState(), stderr or [])) + return session, events + + +def test_an_outage_parks_the_turn_instead_of_ending_it(monkeypatch): + session, events = p_drive(monkeypatch, ConnectionError("Connection reset by peer")) + assert session.pending_continuation is True, "the work is queued to continue" + assert session.pending_continuation_delay_s == RECONNECT_BACKOFFS[0] + assert session.awaiting_reconnect is True + assert session.needs_fresh_session is True, "the CLI died with the outage; resume on a fresh one" + assert not [m for m in session.messages if m.role == "system"], "no card: nothing is over yet" + assert "agent:reconnect_wait" in [e for e, _ in events] + assert "agent:rate_limited" not in [e for e, _ in events] + + +def test_the_wait_widens_and_then_concedes(monkeypatch): + session = p_session() + for expected in RECONNECT_BACKOFFS: + session.pending_continuation = False + p_drive(monkeypatch, ConnectionError("network is unreachable"), session=session) + assert session.pending_continuation_delay_s == expected + + # Budget spent: the honest pill fires rather than a fourth, longer silence. + session.pending_continuation = False + session, events = p_drive(monkeypatch, ConnectionError("network is unreachable"), session=session) + assert session.pending_continuation is False + assert "agent:rate_limited" in [e for e, _ in events] + assert session.status == "completed" + + +def test_a_provider_reset_hint_outranks_our_schedule_but_is_capped(): + s = p_session() + assert arm_reconnect_resume(s, retry_after_s=600) == 605 + s2 = p_session() + assert arm_reconnect_resume(s2, retry_after_s=99999) == RECONNECT_MAX_DELAY_S + + +def test_a_shorter_hint_never_shrinks_the_wait(): + """Negative control: a 1s hint during a real outage must not become a 1s hot loop.""" + s = p_session() + assert arm_reconnect_resume(s, retry_after_s=1) == RECONNECT_BACKOFFS[0] + + +def test_an_armed_continuation_is_never_stomped(): + """Negative control: something else already queued the next turn, so this must stand down.""" + s = p_session() + s.pending_continuation = True + s.pending_continuation_prompt = "someone else's continuation" + assert arm_reconnect_resume(s) is None + assert s.pending_continuation_prompt == "someone else's continuation" + + +def test_a_turn_that_got_through_returns_the_budget(): + s = p_session() + arm_reconnect_resume(s) + assert s.awaiting_reconnect is True + clear_reconnect_wait(s) + assert s.awaiting_reconnect is False + + +def test_quitting_mid_wait_leaves_an_owed_turn(monkeypatch, tmp_path): + """The whole point of persisting the flag: the app going down DURING the wait must not be how + a task quietly evaporates. Boot-restore has to see it, even though the status says completed.""" + from backend.apps.agents.agent_manager import AgentManager + import backend.apps.agents.manager.session.SessionPersistence as sp + + parked = { + "id": "sess-parked", "status": "completed", "awaiting_reconnect": True, + "closed_at": None, "active_branch_id": "main", + "messages": [{"role": "user", "content": "go", "branch_id": "main"}], + } + saved = {} + monkeypatch.setattr(sp, "load_all_session_data", lambda: [("sess-parked", parked)], raising=True) + monkeypatch.setattr(sp, "save_session", lambda sid, data: saved.update({sid: data}), raising=True) + + mgr = AgentManager() + asyncio.run(mgr.reconcile_on_startup()) + assert "sess-parked" in mgr.crash_resume_queue, "a parked turn is an owed turn" + assert saved["sess-parked"]["awaiting_reconnect"] is False, "the flag is consumed, not left to re-fire" + + +def test_the_breaker_stops_a_task_that_keeps_killing_the_app(monkeypatch): + """If the work itself is what takes the process down, a second boot must hand it to the manual + chip rather than launching it again.""" + import backend.apps.agents.manager.session.SessionPersistence as sp + from backend.apps.agents.agent_manager import AgentManager + + parked = { + "id": "sess-bad", "status": "completed", "awaiting_reconnect": True, + "closed_at": None, "active_branch_id": "main", "crash_interrupt_count": 1, + "messages": [{"role": "user", "content": "go", "branch_id": "main"}], + } + monkeypatch.setattr(sp, "load_all_session_data", lambda: [("sess-bad", parked)], raising=True) + monkeypatch.setattr(sp, "save_session", lambda sid, data: None, raising=True) + + mgr = AgentManager() + asyncio.run(mgr.reconcile_on_startup()) + assert mgr.crash_resume_queue == [], "second consecutive death: no third automatic run" + + +def test_the_wait_ends_the_moment_the_provider_answers(monkeypatch): + """The backoff is a CEILING, not a sentence. A blind sleep would leave someone watching a + spinner for fourteen more minutes after their wifi came back, which is worse than the button it + replaced (Eric, 2026-08-20).""" + import backend.apps.agents.manager.run.reconnect_resume as rr + + calls = {"probes": 0, "slept": 0.0} + + async def fake_sleep(secs, *a, **k): + calls["slept"] += secs + + async def reachable_on_third_look(host): + calls["probes"] += 1 + return calls["probes"] >= 3 + + monkeypatch.setattr(rr.asyncio, "sleep", fake_sleep, raising=False) + monkeypatch.setattr(rr, "provider_reachable", reachable_on_third_look, raising=True) + + s = p_session() + asyncio.run(rr.wait_for_reconnect(s, 900)) + assert calls["probes"] == 3, "it stops looking as soon as the answer is yes" + assert calls["slept"] == 3 * rr.RECONNECT_POLL_S, "it waited 9s of a 900s ceiling" + + +def test_an_outage_that_never_heals_still_honours_the_ceiling(monkeypatch): + """Negative control: if nothing ever answers, the wait must END at the ceiling and try anyway, + not poll forever.""" + import backend.apps.agents.manager.run.reconnect_resume as rr + + slept = {"total": 0.0} + + async def fake_sleep(secs, *a, **k): + slept["total"] += secs + + async def never(host): + return False + + monkeypatch.setattr(rr.asyncio, "sleep", fake_sleep, raising=False) + monkeypatch.setattr(rr, "provider_reachable", never, raising=True) + + asyncio.run(rr.wait_for_reconnect(p_session(), 60)) + assert slept["total"] == 60, "the ceiling is honoured exactly, not overshot" + + +def test_the_probe_targets_the_provider_not_our_own_localhost_router(): + """A router lane points the CLI at localhost, which answers happily while the machine is + offline: probing it would return the one wrong answer at the only moment it matters.""" + import backend.apps.agents.manager.run.reconnect_resume as rr + + s = p_session() + for model, expected in ( + ("sonnet-5", "api.anthropic.com"), + ("gpt-5.6-terra", "api.openai.com"), + ("gemini-3-pro", "generativelanguage.googleapis.com"), + ): + s.model = model + host = rr.provider_probe_host(s) + assert host == expected, f"{model} -> {host}" + assert "localhost" not in host and "127.0.0.1" not in host + + +def test_a_dead_socket_respawns_the_cli_but_a_429_does_not(monkeypatch): + """Both arrive as 'transient', and they want different recoveries. A dead transport leaves the + CLI holding a corpse, so the retry needs a fresh one. A 429 is a healthy pipe carrying a NO, and + respawning for that spends a whole process to be told the same thing (caught by the existing + test_rate_limit_does_not_respawn_the_cli when this shipped ungated).""" + dead, _ = p_drive(monkeypatch, ConnectionError("Connection reset by peer")) + assert dead.needs_fresh_session is True + assert dead.awaiting_reconnect is True + + throttled, _ = p_drive(monkeypatch, Exception("429 rate_limit_error: overloaded")) + assert throttled.needs_fresh_session is False, "a refusal is not a broken pipe" + assert throttled.awaiting_reconnect is True, "but it is still worth waiting out" diff --git a/frontend/src/app/pages/AgentChat/AgentChat.tsx b/frontend/src/app/pages/AgentChat/AgentChat.tsx index f354ff7f..fc0cb511 100644 --- a/frontend/src/app/pages/AgentChat/AgentChat.tsx +++ b/frontend/src/app/pages/AgentChat/AgentChat.tsx @@ -69,7 +69,7 @@ import { isShowUiPair, isAskUiPair, extractPendingAskUi, callToolUseId, resultTo import { composerPlaceholder } from './composerPlaceholder'; import ApprovalBar, { BatchApprovalBar } from './shell/ApprovalBar'; import ForceStopAgentBar from './ForceStopAgentBar'; -import { ProviderRetryPill, RateLimitPill } from './shell/RateLimitPill'; +import { ProviderRetryPill, RateLimitPill, ReconnectWaitPill } from './shell/RateLimitPill'; import { ContextRecoveredPill } from './shell/ContextRecoveredPill'; import ChatInput, { ChatInputHandle } from './ChatInput'; import FollowupChips from './FollowupChips'; @@ -2111,6 +2111,7 @@ const AgentChat: React.FC = ({ sessionId: sessionIdProp, onClose )} + diff --git a/frontend/src/app/pages/AgentChat/shell/RateLimitPill.tsx b/frontend/src/app/pages/AgentChat/shell/RateLimitPill.tsx index 4932a343..0c0098b7 100644 --- a/frontend/src/app/pages/AgentChat/shell/RateLimitPill.tsx +++ b/frontend/src/app/pages/AgentChat/shell/RateLimitPill.tsx @@ -99,3 +99,46 @@ export const RateLimitPill: React.FC<{ sessionId: string }> = ({ sessionId }) => ); }; + +/** The connection dropped for longer than the in-turn retry ladder covers, so the turn is PARKED + * rather than finished: it wakes itself on a widening schedule and continues where it left off. + * This is the one pill that must NOT auto-clear on a short timer, because the wait it describes can + * be fifteen minutes; an agent sitting silent that long is exactly what makes people force-quit and + * lose the task. It clears when the next turn actually lands. */ +export const ReconnectWaitPill: React.FC<{ sessionId: string }> = ({ sessionId }) => { + const c = useClaudeTokens(); + const rw = useAppSelector((s) => s.agents.sessions[sessionId]?.reconnect_wait); + + const label = (() => { + const secs = rw?.retry_in_s ?? 0; + if (!secs) return 'Connection lost, retrying'; + const mins = Math.round(secs / 60); + return mins >= 1 ? `Connection lost, retrying in ~${mins} min` : 'Connection lost, retrying shortly'; + })(); + const lastLabel = useRef(label); + if (rw) lastLabel.current = label; + + return ( + + + + {lastLabel.current} + + + ); +}; diff --git a/frontend/src/shared/state/agentsSlice.ts b/frontend/src/shared/state/agentsSlice.ts index a0620436..1314be4e 100644 --- a/frontend/src/shared/state/agentsSlice.ts +++ b/frontend/src/shared/state/agentsSlice.ts @@ -118,6 +118,8 @@ export interface AgentSession { framework_overhead_tokens?: number; context_overflow?: { reason: string; message: string; at: string } | null; rate_limited?: { retry_after_s: number | null; at: string } | null; + // Parked waiting for the connection back; unlike the pills above this can last minutes, so the UI has to say so. + reconnect_wait?: { retry_in_s: number | null; attempt: number | null; at: string } | null; provider_retrying?: { attempt: number | null; delay_ms: number | null; at: string } | null; context_recovered?: { at: string } | null; // Set when a view-builder turn installed/changed deps, so the app card does a HARD reload (Vite restart) at turn-finish instead of the soft one. Reset when the next turn starts. @@ -1051,6 +1053,25 @@ const agentsSlice = createSlice({ if (session) session.rate_limited = null; }, + setReconnectWait( + state, + action: PayloadAction<{ sessionId: string; retryInS: number | null; attempt: number | null }> + ) { + const session = state.sessions[action.payload.sessionId]; + if (session) { + session.reconnect_wait = { + retry_in_s: action.payload.retryInS, + attempt: action.payload.attempt, + at: new Date().toISOString(), + }; + } + }, + + clearReconnectWait(state, action: PayloadAction<{ sessionId: string }>) { + const session = state.sessions[action.payload.sessionId]; + if (session) session.reconnect_wait = null; + }, + setAppDepsChanged(state, action: PayloadAction<{ sessionId: string }>) { const session = state.sessions[action.payload.sessionId]; if (session) session.app_deps_changed = true; @@ -1543,6 +1564,8 @@ export const { setContextOverflow, setRateLimited, clearRateLimited, + setReconnectWait, + clearReconnectWait, setProviderRetrying, clearProviderRetrying, setContextRecovered, diff --git a/frontend/src/shared/ws/WebSocketManager.ts b/frontend/src/shared/ws/WebSocketManager.ts index 1cdd37e6..7af40420 100644 --- a/frontend/src/shared/ws/WebSocketManager.ts +++ b/frontend/src/shared/ws/WebSocketManager.ts @@ -14,6 +14,8 @@ import { updateSessionContext, setContextOverflow, setRateLimited, + setReconnectWait, + clearReconnectWait, setProviderRetrying, setContextRecovered, setAppDepsChanged, @@ -474,6 +476,8 @@ class WebSocketManager { } if (data.status === 'running' && session_id) { store.dispatch(trackAgentNotification(session_id)); + // The parked-for-reconnect pill describes a wait that just ended; leaving it up outlives the recovery it was announcing. + store.dispatch(clearReconnectWait({ sessionId: session_id })); } // Native OS notification when an agent finishes while the user is elsewhere: workflows already had this; long chat tasks deserve the same "it's done" tap on both platforms. Sub-agents stay silent (their parent's finish is the story). if (data.status === 'completed' && session_id && document.hidden) { @@ -715,6 +719,17 @@ class WebSocketManager { } break; + case 'agent:reconnect_wait': + // The turn is PARKED, not over: it retries itself on a widening schedule, and an agent that looks idle for fifteen minutes reads as broken. + if (session_id) { + store.dispatch(setReconnectWait({ + sessionId: session_id, + retryInS: typeof data.retry_in_s === 'number' ? data.retry_in_s : null, + attempt: typeof data.attempt === 'number' ? data.attempt : null, + })); + } + break; + case 'agent:context_recovered': // The backend hit a context-overflow crash mid-turn, rebuilt from its local copy, and retried on its own. Transient muted pill so the recovery is visible without reading like an error. if (session_id) {