[eric] agents: preflight bounces the router but never declares a credential dead, because it has not dispatched (ENG-414)

backend/apps/agents/manager/run/lane_preflight.py

@@ -13,9 +13,17 @@ So this looks first, and it tries to fix it before it complains:
                         `unavailable` stamp and modelLock cooldowns), then DISPATCH ANYWAY and let
                         the turn itself be the verdict. If it goes through, the user never learns
                         anything happened.
-  sticky-dead, again -> one accurate sentence, immediately. No invented rotation window, no wait
-                        the user has already tried by hand, no promise of a self-heal that cannot
-                        happen.
+  sticky-dead, bounce
+  already throttled  -> DISPATCH ANYWAY too, and flag the session so that if the turn really does
+                        401, handle_run_error can say the accurate sentence with no rotation story.
+
+This file NEVER tells the user a credential is dead, because it has not dispatched and therefore
+cannot know. It used to, off the bounce cooldown, and that cooldown is a GLOBAL router-restart
+throttle: the branch that meant "permanently dead" actually meant "another chat restarted the
+router in the last five minutes". It killed a live build on a working credential while telling the
+user "waiting will not clear this one", which was backwards, since waiting out the throttle is
+exactly what cleared it (ENG-414). The death verdict lives in ONE place now, downstream of a real
+failed dispatch.
 
 The bounce is NOT allowed to declare success on its own, and that mistake is worth recording: the
 first version re-read the health flag afterwards and called a cleared stamp a recovery. But a fresh
@@ -108,16 +116,15 @@ async def preflight_lane(resolved_model: str,
         return None
 
     dead = await dead_connection(provider)
-    if dead is None:
-        return None
-
-    # Whatever happens next, an auth failure on THIS turn is a dead credential, not a rotation
-    # window: the router had already given up before we sent anything.
+    # Written on EVERY pass, both directions. It used to be latched True and never cleared, so one
+    # blip made every later auth error in that session claim a permanently dead credential.
     if session is not None:
         try:
-            session.lane_credential_dead = True
+            session.lane_credential_dead = dead is not None
         except Exception:
             pass
+    if dead is None:
+        return None
 
     now = time.time()
     if now - LAST_BOUNCE.get(provider, 0.0) >= BOUNCE_COOLDOWN_S:
@@ -141,8 +148,14 @@ async def preflight_lane(resolved_model: str,
         # Deliberately no post-bounce health re-read: see the module docstring. Dispatch is the test.
         return None
 
-    return RECONNECT_COPY.get(
-        provider,
-        "This model's sign-in expired and could not be renewed. Reconnect it in Settings, then "
-        "Models. Waiting will not clear this one.",
+    # The bounce was throttled, and that says NOTHING about this credential: LAST_BOUNCE is global,
+    # so the timer belongs to whichever OTHER chat restarted the router last. Carding here declared
+    # a working lane dead and killed a live build (ENG-414), and it broke this file's own rule that
+    # only a real dispatch can decide. So dispatch. If the credential really is gone, the turn 401s
+    # and handle_run_error shows the accurate card immediately off `lane_credential_dead`, which is
+    # the same sentence this used to return, minus the guessing.
+    logger.info(
+        f"lane preflight: {provider} looks dead but the router bounce is throttled; dispatching "
+        "anyway and letting the turn decide"
     )
+    return None

backend/tests/test_lane_preflight.py

@@ -78,18 +78,30 @@ def test_a_cleared_stamp_is_never_mistaken_for_a_working_credential(monkeypatch)
     )
 
 
-def test_a_lane_still_dead_on_the_next_ask_gets_one_accurate_sentence(monkeypatch):
-    """Second encounter inside the cooldown: we already spent a bounce and a turn, so stop
-    pretending and say the true thing."""
+def test_a_lane_still_dead_inside_the_cooldown_DISPATCHES(monkeypatch):
+    """CORRECTED 2026-08-27 (ENG-414). This used to assert the opposite, and the assumption it
+    encoded is the bug: "second encounter inside the cooldown" was read as "we already spent a
+    bounce and a turn on THIS session". `LAST_BOUNCE` is module-global, so in production it meant
+    "some other chat bounced recently" and it hard-stopped a live build on a working credential.
+
+    Preflight has not dispatched, so it cannot know. It dispatches and flags the session; the
+    accurate sentence now comes from handle_run_error after a real 401. The cooldown still holds."""
     st = p_providers(monkeypatch, P_DEAD, bounce_result=P_DEAD)
     assert asyncio.run(lp.preflight_lane("cx/gpt-5.6")) is None
-    msg = asyncio.run(lp.preflight_lane("cx/gpt-5.6"))
-    assert msg and "ChatGPT" in msg and "Reconnect" in msg
-    assert "rotated" not in msg.lower(), "never claim a rotation that did not happen"
-    assert "no action needed" not in msg.lower(), "there IS action needed; saying otherwise is the bug"
+    assert asyncio.run(lp.preflight_lane("cx/gpt-5.6")) is None, \
+        "a throttled bounce is not evidence about the credential"
     assert st["bounced"] == 1, "the cooldown holds; one restart, not one per ask"
 
 
+def test_the_downstream_card_still_refuses_the_rotation_story():
+    """What the old assertion above was really protecting: when the card DOES fire, it must not
+    invent a rotation window or claim no action is needed. That copy moved, it did not soften."""
+    for msg in lp.RECONNECT_COPY.values():
+        assert "rotated" not in msg.lower(), "never claim a rotation that did not happen"
+        assert "no action needed" not in msg.lower(), "there IS action needed; saying otherwise is the bug"
+        assert "reconnect" in msg.lower(), msg
+
+
 def test_the_bounce_is_rate_limited(monkeypatch):
     """A bounce restarts a process every other session shares, so it is once per lane per window, never once per turn."""
     st = p_providers(monkeypatch, P_DEAD, bounce_result=P_DEAD)

backend/tests/test_lane_preflight_never_declares_death.py

@@ -0,0 +1,118 @@
+"""Preflight may bounce the router; it may never say a credential is dead. It has not dispatched.
+
+ENG-414, production 1.7.9, hit twice in one session and the second one killed a live app build.
+`LAST_BOUNCE` is a MODULE-LEVEL dict keyed by provider, and `BOUNCE_COOLDOWN_S` is 300, so the
+branch that meant "this credential is permanently dead" actually meant "some OTHER chat restarted
+the router in the last five minutes". The user was told "Waiting will not clear this one" and then
+proved it wrong by typing "continue" a minute later and watching the run finish.
+"""
+
+import time
+
+import pytest
+
+from backend.apps.agents.core.models import AgentSession
+from backend.apps.agents.manager.run import lane_preflight as p_pf
+
+SRC = "backend/apps/agents/manager/run/lane_preflight.py"
+
+
+def p_session() -> AgentSession:
+    return AgentSession(id="s1", name="c", title="c", model="cc/claude-opus-5")
+
+
+@pytest.fixture(autouse=True)
+def p_clean():
+    p_pf.LAST_BOUNCE.clear()
+    yield
+    p_pf.LAST_BOUNCE.clear()
+
+
+@pytest.mark.asyncio
+async def test_a_throttled_bounce_dispatches_instead_of_carding(monkeypatch):
+    """The exact shape that killed the build: another chat bounced 10s ago, this lane reads dead."""
+    monkeypatch.setattr(p_pf, "dead_connection",
+                        lambda provider: p_async({"testStatus": "unavailable", "errorCode": 401}))
+    p_pf.LAST_BOUNCE["claude"] = time.time() - 10      # another chat, well inside the 300s window
+    s = p_session()
+    assert await p_pf.preflight_lane("cc/claude-opus-5", s) is None, \
+        "a throttled bounce says nothing about the credential; the turn must still be spent"
+
+
+@pytest.mark.asyncio
+async def test_the_session_is_flagged_so_a_REAL_401_can_still_be_honest(monkeypatch):
+    """Dispatching anyway must not lose the accuracy the old card had."""
+    monkeypatch.setattr(p_pf, "dead_connection",
+                        lambda provider: p_async({"testStatus": "unavailable", "errorCode": 401}))
+    p_pf.LAST_BOUNCE["claude"] = time.time() - 10
+    s = p_session()
+    await p_pf.preflight_lane("cc/claude-opus-5", s)
+    assert s.lane_credential_dead is True, "handle_run_error keys its accurate card on this"
+
+
+@pytest.mark.asyncio
+async def test_a_healthy_lane_CLEARS_the_flag_instead_of_latching_it(monkeypatch):
+    """It was set True and never reset anywhere, so one blip made every later auth error in that
+    session claim a permanently dead credential."""
+    s = p_session()
+    s.lane_credential_dead = True
+    monkeypatch.setattr(p_pf, "dead_connection", lambda provider: p_async(None))
+    assert await p_pf.preflight_lane("cc/claude-opus-5", s) is None
+    assert s.lane_credential_dead is False, "the flag is a live fact, not a latch"
+
+
+@pytest.mark.asyncio
+async def test_the_death_verdict_exists_in_exactly_one_place(monkeypatch):
+    """Two code paths for one verdict is how they disagreed. Only the one downstream of a real
+    failed dispatch may say it."""
+    src = open(SRC).read()
+    i = src.index("def preflight_lane")
+    body = src[i:]
+    assert "RECONNECT_COPY.get(" not in body, "preflight cannot know; it has not dispatched"
+    handler = open("backend/apps/agents/manager/run/handle_run_error.py").read()
+    assert "RECONNECT_COPY.get(" in handler, "the accurate card must survive downstream"
+    assert 'getattr(session, "lane_credential_dead", False)' in handler, \
+        "and it must still be gated on the router having given up BEFORE the turn"
+
+
+@pytest.mark.asyncio
+async def test_the_bounce_itself_is_still_throttled(monkeypatch):
+    """A bounce restarts a process every session shares. Dispatching anyway must NOT turn into
+    bouncing on every turn."""
+    p_calls = []
+
+    async def p_fake_bounce(provider):
+        p_calls.append(provider)
+        return True
+
+    monkeypatch.setattr(p_pf, "dead_connection",
+                        lambda provider: p_async({"testStatus": "unavailable", "errorCode": 401}))
+    import backend.apps.nine_router.bounce_after_connect as p_b
+    monkeypatch.setattr(p_b, "bounce_router_after_connect", p_fake_bounce)
+    s = p_session()
+    await p_pf.preflight_lane("cc/claude-opus-5", s)     # first: allowed to bounce
+    await p_pf.preflight_lane("cc/claude-opus-5", s)     # second: throttled
+    await p_pf.preflight_lane("cc/claude-opus-5", s)     # third: throttled
+    assert len(p_calls) == 1, f"one bounce per {p_pf.BOUNCE_COOLDOWN_S}s window, got {len(p_calls)}"
+
+
+@pytest.mark.asyncio
+async def test_a_router_that_does_not_come_back_still_stops_the_turn(monkeypatch):
+    """The one thing preflight CAN know without dispatching: it just restarted the router and the
+    router is not listening. Dispatching into that is a guaranteed connection error the user would
+    read as the model failing."""
+    monkeypatch.setattr(p_pf, "dead_connection",
+                        lambda provider: p_async({"testStatus": "unavailable", "errorCode": 401}))
+    import backend.apps.nine_router.bounce_after_connect as p_b
+
+    async def p_dead_bounce(provider):
+        return False
+
+    monkeypatch.setattr(p_b, "bounce_router_after_connect", p_dead_bounce)
+    msg = await p_pf.preflight_lane("cc/claude-opus-5", p_session())
+    assert msg and "restarting" in msg
+    assert "Waiting will not clear" not in msg, "a restarting router is not a dead credential"
+
+
+async def p_async(value):
+    return value