mirror of
https://github.com/openswarm-ai/openswarm.git
synced 2026-09-26 19:44:51 +02:00
[eric] agents: no turn ends silent, and a codex 401 waits out the rotation window
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014wtspwSFzZmjCx9UNPAorQ
This commit is contained in:
co-authored by
Claude Opus 5
parent
cc854d9c0f
commit
d5934b1da1
@@ -61,6 +61,20 @@ P_SUBSCRIPTION_STATE_PATTERNS = re.compile(
|
||||
|
||||
AUTH_RESUME_WAIT_CAP = 120
|
||||
|
||||
# A codex/GPT subscription token rotates every 1-2 minutes; anything shorter than the window just
|
||||
# retries into the same expiry.
|
||||
CODEX_ROTATION_RESUME_WAIT = 75
|
||||
|
||||
P_CODEX_ROTATION_PATTERNS = re.compile(
|
||||
r"(?:\[?codex/|\bcx/|\bgpt-[0-9])"
|
||||
r".*?"
|
||||
r"(?:authentication\s+token\s+(?:is|has)\s+expired|token\s+expired|\b401\b)"
|
||||
r"|(?:authentication\s+token\s+(?:is|has)\s+expired|token\s+expired|\b401\b)"
|
||||
r".*?"
|
||||
r"(?:\[?codex/|\bcx/|\bgpt-[0-9])",
|
||||
re.IGNORECASE | re.DOTALL,
|
||||
)
|
||||
|
||||
|
||||
@typechecked
|
||||
def auth_resume_wait(exc: BaseException, attempt: int, extra_text: str = "") -> Optional[int]:
|
||||
@@ -95,6 +109,12 @@ def auth_resume_wait(exc: BaseException, attempt: int, extra_text: str = "") ->
|
||||
hinted = parse_retry_after(exc, extra_text)
|
||||
if hinted is not None:
|
||||
return min(hinted + 5, AUTH_RESUME_WAIT_CAP)
|
||||
# Codex tokens rotate on a 1-2 minute cadence, so a 20s resume lands back INSIDE the same
|
||||
# window and spends the one attempt on a failure that was always going to fail. This is the
|
||||
# turn-level twin of the ENG-361 wait: without it that fix never gets a say, because this
|
||||
# retry runs first (drill C5/D6, 2026-08-20).
|
||||
if P_CODEX_ROTATION_PATTERNS.search(combined):
|
||||
return CODEX_ROTATION_RESUME_WAIT
|
||||
return 20
|
||||
|
||||
|
||||
|
||||
@@ -47,12 +47,22 @@ def maybe_nudge_empty_finish(session: AgentSession, session_id: str) -> bool:
|
||||
if getattr(session, "pending_continuation", False):
|
||||
return False
|
||||
if not turn_finished_empty(session):
|
||||
# A turn that produced NOTHING at all (no text, no tool call) leaves the same Done pill
|
||||
# over an empty chat, and the tail-walk above can't see it: with nothing persisted the
|
||||
# tail is still the user's own message. Nudging it would re-send a prompt the model just
|
||||
# refused, so say so instead of ending mute.
|
||||
if p_turn_produced_nothing(session):
|
||||
p_surface_exhausted(session, session_id)
|
||||
return False
|
||||
if session.empty_finish_nudges >= NUDGE_HARD_CAP:
|
||||
p_surface_exhausted(session, session_id)
|
||||
return False
|
||||
p_tool_calls = p_count_tool_calls(session)
|
||||
if session.empty_finish_nudges >= 1 and p_tool_calls <= session.empty_finish_progress_mark:
|
||||
# The nudge bought no new work, so re-nudging would ping-pong a model with nothing left.
|
||||
# Refusing is right; ending the ask in SILENCE is not, and that is what the user actually
|
||||
# reports as "the agent just stopped" (Haik's poke storms).
|
||||
p_surface_exhausted(session, session_id)
|
||||
return False
|
||||
session.empty_finish_progress_mark = p_tool_calls
|
||||
# At high context the silent quit is usually the model choking on the prompt itself, so
|
||||
@@ -126,6 +136,20 @@ def p_surface_exhausted(session: AgentSession, session_id: str) -> None:
|
||||
P_ANSWER_TOOL_MARKERS = ("openswarm-ui", "ShowUI", "AskUI", "AskUserQuestion")
|
||||
|
||||
|
||||
@typechecked
|
||||
def p_turn_produced_nothing(session: AgentSession) -> bool:
|
||||
"""True when the model returned an empty hand: no reply, no tool work, nothing the user can
|
||||
read. Thinking does not count, because a collapsed reasoning trace is not an answer, and a
|
||||
thinking-only end_turn is the exact shape of the quits users report."""
|
||||
msgs: List = [
|
||||
m for m in get_branch_messages(session)
|
||||
if not getattr(m, "hidden", False) and getattr(m, "role", "") != "thinking"
|
||||
]
|
||||
if not msgs or getattr(msgs[-1], "role", "") != "user":
|
||||
return False
|
||||
return not any(getattr(m, "role", "") in ("assistant", "tool_call") for m in msgs)
|
||||
|
||||
|
||||
@typechecked
|
||||
def p_count_tool_calls(session: AgentSession) -> int:
|
||||
return sum(1 for m in get_branch_messages(session) if getattr(m, "role", "") == "tool_call")
|
||||
|
||||
@@ -46,7 +46,14 @@ async def test_first_token_expiry_heals_silently(monkeypatch):
|
||||
with patch.object(assistant_message.ws_manager, "send_to_session", new=fake_send):
|
||||
await assistant_message.handle_assistant_message(
|
||||
p_asst([TextBlock(text=txt)]), session, session.id, turn, thinking, {}, {})
|
||||
assert not any(m.role == "system" for m in session.messages), "no banner on the first expiry"
|
||||
# ENG-361 amended this contract: the codex retry now waits ~75s to clear the rotation window,
|
||||
# and a minute of nothing reads as a hang, so ONE slim "retrying automatically" notice is
|
||||
# expected. What must never appear is the reconnect BANNER, because there is nothing for the
|
||||
# user to do. The distinction is the whole point: a notice informs, a banner assigns homework.
|
||||
p_sys = [m for m in session.messages if m.role == "system"]
|
||||
assert len(p_sys) == 1, "exactly one slim notice, not a wall"
|
||||
assert "no action needed" in p_sys[0].content.lower()
|
||||
assert "reconnect" not in p_sys[0].content.lower(), "never demand a reconnect on the first expiry"
|
||||
assert not any(m.role == "assistant" for m in session.messages)
|
||||
assert "agent:auth_error" not in events
|
||||
assert session.auth_retry_used is True
|
||||
|
||||
@@ -91,3 +91,28 @@ def test_the_recovery_ledger_counts_auth_resumes():
|
||||
src = inspect.getsource(TurnRunner)
|
||||
assert "auth-resume" in src.split("record_recovery", 1)[0] or "p_auth_retry_attempt" in src.split("record_recovery", 1)[1].split(")", 2)[1], \
|
||||
"a survived auth blip must land in the near-miss ledger"
|
||||
|
||||
|
||||
def test_codex_rotation_resume_waits_past_the_rotation_window():
|
||||
"""A codex token rotates every 1-2 minutes, so the turn-level resume has to clear the window.
|
||||
At 20s it retried into the same expiry and spent the single attempt for nothing, which meant
|
||||
the ENG-361 self-heal downstream never got a say (drill D6/C5, 2026-08-20)."""
|
||||
from backend.apps.agents.core.error_classify import CODEX_ROTATION_RESUME_WAIT
|
||||
for text in (
|
||||
"[codex/gpt-5.6] API Error: 401 authentication token is expired",
|
||||
"cx/gpt-5.4: token expired",
|
||||
"API Error 401 on gpt-5.6-terra: authentication token has expired",
|
||||
):
|
||||
assert auth_resume_wait(Exception(text), 0) == CODEX_ROTATION_RESUME_WAIT, text
|
||||
assert CODEX_ROTATION_RESUME_WAIT > 60, "must outlast the rotation window"
|
||||
|
||||
|
||||
def test_non_codex_auth_failures_keep_the_short_resume():
|
||||
"""Negative control: only the rotating lane pays the long wait; a plain bad key must still
|
||||
come back fast, or every Anthropic 401 gets a minute of dead air."""
|
||||
for text in (
|
||||
"API Error: 401 authentication_error invalid x-api-key",
|
||||
"Request failed: 401 Unauthorized",
|
||||
"Invalid bearer token",
|
||||
):
|
||||
assert auth_resume_wait(Exception(text), 0) == 20, text
|
||||
|
||||
@@ -215,3 +215,73 @@ def test_vanishing_quit_on_repeat_session_is_claimed():
|
||||
assert turn_finished_empty(s) is False, "first-time session: user tail stays unclaimed"
|
||||
s.empty_finish_total = 1
|
||||
assert turn_finished_empty(s) is True, "repeat session: the vanishing quit is claimed"
|
||||
|
||||
|
||||
def p_system_lines(session) -> list:
|
||||
return [m for m in session.messages if m.role == "system"]
|
||||
|
||||
|
||||
def test_stalled_continuation_still_tells_the_user() -> None:
|
||||
"""The anti-ping-pong guard is right to refuse a second nudge, but the ask must not end in
|
||||
silence: that Done-pill-over-tool-rows is exactly what users report as 'it just stopped'
|
||||
(drill D3/C4, 2026-08-20)."""
|
||||
s = p_session(("user", "audit the repo"),
|
||||
("tool_call", {"tool": "Bash", "input": {}}),
|
||||
("tool_result", {"text": "ok"}))
|
||||
assert maybe_nudge_empty_finish(s, "sid") is True
|
||||
s.pending_continuation = False
|
||||
# The nudge bought nothing at all.
|
||||
assert maybe_nudge_empty_finish(s, "sid") is False
|
||||
assert s.empty_finish_nudges == 1, "still no second nudge"
|
||||
assert len(p_system_lines(s)) == 1, "but the user is told once"
|
||||
|
||||
# And it stays once, however many stalled quits follow.
|
||||
s.pending_continuation = False
|
||||
assert maybe_nudge_empty_finish(s, "sid") is False
|
||||
assert len(p_system_lines(s)) == 1
|
||||
|
||||
|
||||
def test_turn_that_produced_nothing_is_not_silent() -> None:
|
||||
"""No text, no tool call, nothing persisted: the tail-walk cannot see it, so the honest line
|
||||
is the only thing standing between the user and an empty Done pill (drill D4/C3/C5)."""
|
||||
s = p_session(("user", "read all 16 files and report the magic word"))
|
||||
assert maybe_nudge_empty_finish(s, "sid") is False, "nudging would re-send a refused prompt"
|
||||
assert len(p_system_lines(s)) == 1, "the user gets an honest line instead of silence"
|
||||
|
||||
|
||||
def test_a_working_turn_is_never_given_the_exhausted_line() -> None:
|
||||
"""Negative control: a turn that actually answered must stay clean."""
|
||||
s = p_session(("user", "hi"), ("assistant", "here is your answer"))
|
||||
assert maybe_nudge_empty_finish(s, "sid") is False
|
||||
assert p_system_lines(s) == [], "an answered turn earns no card"
|
||||
|
||||
|
||||
def test_a_turn_still_holding_tool_work_is_not_called_empty_handed() -> None:
|
||||
"""Negative control for the produced-nothing seal: tool work exists, so this is the ordinary
|
||||
nudge path, not the empty-handed one."""
|
||||
s = p_session(("user", "task"),
|
||||
("tool_call", {"tool": "Read", "input": {}}),
|
||||
("tool_result", {"text": "x"}))
|
||||
assert maybe_nudge_empty_finish(s, "sid") is True, "ordinary silent quit still nudges"
|
||||
assert p_system_lines(s) == [], "and says nothing yet, because work may still land"
|
||||
|
||||
|
||||
def test_the_honest_lines_survive_the_frontends_jargon_filter() -> None:
|
||||
"""The seal only works if the user can SEE the line. MessageBubble.tsx deliberately swallows
|
||||
raw subprocess/API dumps rendered as system messages, so an honest note that happens to match
|
||||
those patterns would be added by the backend and then silently dropped by the UI: the exact
|
||||
silence this whole issue is about, just moved one layer up. Kept here rather than in a .tsx
|
||||
test so it lives beside the text it guards and cannot drift from it."""
|
||||
import re
|
||||
from backend.apps.agents.manager.run.empty_finish import EXHAUSTED_NOTE
|
||||
|
||||
# Mirrors the swallow test in frontend/src/app/pages/AgentChat/bubbles/MessageBubble.tsx.
|
||||
p_swallowed = re.compile(
|
||||
r'Command failed with exit code|API Error:|invalid_request_error'
|
||||
r'|"type"\s*:\s*"error"|Check stderr output',
|
||||
re.IGNORECASE,
|
||||
)
|
||||
assert not p_swallowed.search(EXHAUSTED_NOTE), (
|
||||
"the exhausted note would be swallowed by the UI's dev-jargon filter"
|
||||
)
|
||||
assert EXHAUSTED_NOTE.strip(), "an empty note renders as nothing at all"
|
||||
|
||||
@@ -10,7 +10,7 @@ from backend.apps.agents.manager.run.handle_run_error import handle_run_error
|
||||
from backend.apps.agents.manager.streaming.state import TurnState
|
||||
|
||||
|
||||
def p_drive_error(monkeypatch, exc, stderr=None):
|
||||
def p_drive_error(monkeypatch, exc, stderr=None, session=None):
|
||||
events = []
|
||||
|
||||
async def fake_send(session_id, event, data):
|
||||
@@ -20,7 +20,10 @@ def p_drive_error(monkeypatch, exc, stderr=None):
|
||||
# Diagnostics are fire-and-forget network; keep the tests offline.
|
||||
import backend.apps.service.client as service_client
|
||||
monkeypatch.setattr(service_client, "submit_diagnostic", lambda payload: None, raising=True)
|
||||
session = AgentSession(name="t", model="sonnet", dashboard_id="d")
|
||||
# Passing a session back in drives a SECOND failure on the same ask, which is the only way to
|
||||
# test a once-per-ask budget: a fresh session would silently hand it a fresh budget too.
|
||||
if session is None:
|
||||
session = AgentSession(name="t", model="sonnet", dashboard_id="d")
|
||||
asyncio.run(handle_run_error(exc, session, session.id, TurnState(), stderr or []))
|
||||
return session, events
|
||||
|
||||
@@ -113,9 +116,29 @@ def test_out_of_credits_does_not_respawn_the_cli(monkeypatch):
|
||||
assert session.needs_fresh_session is False
|
||||
|
||||
|
||||
def test_auth_failure_does_not_respawn_the_cli(monkeypatch):
|
||||
def test_auth_failure_self_heals_once_then_stops_respawning(monkeypatch):
|
||||
# ENG-361 amended the older "never respawn on auth" rule: every sub lane now gets ONE silent
|
||||
# self-heal before any card, and a fresh CLI is exactly how the stale token gets dropped. The
|
||||
# rule that still matters is that it happens ONCE; a credential that fails twice is genuinely
|
||||
# dead, and respawning forever would just hide it behind an endless retry.
|
||||
session, _ = p_drive_error(monkeypatch, Exception("401 invalid authentication credentials"))
|
||||
assert session.needs_fresh_session is True, "one rebuild is the heal"
|
||||
assert session.auth_retry_used is True
|
||||
assert not [m for m in session.messages if m.role == "system"], "no card on the first expiry"
|
||||
|
||||
session.needs_fresh_session = False
|
||||
session.pending_continuation = False
|
||||
p_drive_error(monkeypatch, Exception("401 invalid authentication credentials"), session=session)
|
||||
assert session.needs_fresh_session is False, "the budget is spent; stop respawning"
|
||||
assert [m for m in session.messages if m.role == "system"], "the second failure is honest"
|
||||
|
||||
|
||||
def test_missing_credential_never_burns_a_retry(monkeypatch):
|
||||
# Negative control: a config problem retries identically, so it must card immediately instead
|
||||
# of spending a rebuild and a wait on a request that cannot succeed.
|
||||
session, _ = p_drive_error(monkeypatch, Exception("No credentials for provider: claude (401)"))
|
||||
assert session.needs_fresh_session is False
|
||||
assert [m for m in session.messages if m.role == "system"], "straight to the honest card"
|
||||
|
||||
|
||||
def test_rate_limit_does_not_respawn_the_cli(monkeypatch):
|
||||
|
||||
@@ -5,6 +5,7 @@ coverage), so it pins the observable contract: streamed text lands as an assista
|
||||
tool calls are recorded, and the turn completes."""
|
||||
|
||||
import asyncio
|
||||
import os
|
||||
|
||||
import pytest
|
||||
import claude_agent_sdk
|
||||
@@ -181,6 +182,14 @@ def test_loop_builds_direct_anthropic_key_env(monkeypatch):
|
||||
import backend.apps.agents.agent_manager as am
|
||||
import backend.apps.agents.manager.run.RunOptions as run_opts
|
||||
|
||||
# Supply the packaged shape this strip exists for: a bundle site-packages entry that must be
|
||||
# dropped, next to the debugger dir that must survive.
|
||||
monkeypatch.setenv(
|
||||
"PYTHONPATH",
|
||||
os.pathsep.join(["/Applications/OpenSwarm.app/Contents/Resources/python-env/lib/site-packages",
|
||||
"/opt/openswarm/debugger"]),
|
||||
)
|
||||
|
||||
settings = AppSettings(anthropic_api_key="sk-ant-test123", connection_mode="own_key")
|
||||
monkeypatch.setattr(am, "load_settings", lambda: settings, raising=True)
|
||||
monkeypatch.setattr(run_opts, "load_settings", lambda: settings, raising=True)
|
||||
@@ -208,6 +217,13 @@ def test_loop_builds_direct_anthropic_key_env(monkeypatch):
|
||||
asyncio.run(mgr.run_agent_loop(session.id, "hi"))
|
||||
|
||||
env = captured["options"].env
|
||||
# PYTHONPATH rides along stripped of site-packages (ENG-347: the bundle shadowed agent venvs).
|
||||
# The loop only forwards a PYTHONPATH the ambient shell actually set, so the fixture below
|
||||
# supplies one; without it this asserted whatever the developer's terminal happened to export
|
||||
# and failed on any machine that exported nothing.
|
||||
p_pp = env.pop("PYTHONPATH", None)
|
||||
assert p_pp is not None and "site-packages" not in p_pp
|
||||
assert "debugger" in p_pp, "the debugger injection dir is the one entry that survives"
|
||||
# Direct key, no 9router proxy; the CLI's own auto-memory is force-disabled on every spawn (ENG-222).
|
||||
assert env == {"ANTHROPIC_API_KEY": "sk-ant-test123", "CLAUDE_CODE_DISABLE_AUTO_MEMORY": "1"}
|
||||
|
||||
|
||||
@@ -887,6 +887,7 @@ const MessageBubble: React.FC<Props> = React.memo((props) => {
|
||||
const { body: sysBody, note: sysNote } = extractPlatformNote(rawSysText);
|
||||
const sysText = sysNote || sysBody;
|
||||
// A raw subprocess/API failure ("Command failed with exit code 1", API Error JSON) is dev jargon, and the same failure is already shown as a friendly card on the assistant side. Swallow just that stderr dump so the user sees one calm card, not jargon beneath it.
|
||||
// Widen this at your peril: the silent-quit seal's honest lines ride the same system role, and swallowing one turns a stopped agent back into an unexplained Done pill. backend/tests/test_empty_finish.py pins that they survive.
|
||||
if (/Command failed with exit code|API Error:|invalid_request_error|"type"\s*:\s*"error"|Check stderr output/i.test(sysText)) {
|
||||
return null;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user