Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions agent/error_classifier.py
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,20 @@ def is_auth(self) -> bool:
"signature", # Combined with "thinking" check
]

# Message-string patterns that indicate a provider-side timeout even when
# the exception type is generic (e.g. RuntimeError from a local shim that
# wraps a subprocess timeout). Checked before the type-based transport
# heuristics so custom-provider "timed out" errors don't fall through to
# the unknown bucket and get misreported as empty responses.
_TIMEOUT_MESSAGE_PATTERNS = [
"timed out",
"turn timed out",
"request timed out",
"deadline exceeded",
"operation timed out",
"upstream timed out",
]

# Transport error type names
_TRANSPORT_ERROR_TYPES = frozenset({
"ReadTimeout", "ConnectTimeout", "PoolTimeout",
Expand Down Expand Up @@ -963,6 +977,14 @@ def _classify_by_message(
should_fallback=True,
)

# Timeout message patterns — generic exception types (e.g. RuntimeError)
# raised by local shims or custom providers that internally wrap a
# subprocess/HTTP timeout. Classified as transport timeout so the retry
# loop rebuilds the client instead of treating the turn as an empty
# model response.
if any(p in error_msg for p in _TIMEOUT_MESSAGE_PATTERNS):
return result_fn(FailoverReason.timeout, retryable=True)

return None


Expand Down
38 changes: 38 additions & 0 deletions run_agent.py
Original file line number Diff line number Diff line change
Expand Up @@ -3529,6 +3529,19 @@ def _extract_reasoning(self, assistant_message) -> Optional[str]:
# instead of returning structured reasoning fields. Only fall back
# to inline extraction when no structured reasoning was found.
content = getattr(assistant_message, "content", None)
if not reasoning_parts and isinstance(content, list):
# DeepSeek V4 Pro (and compatible providers) return content as a
# list of typed blocks, e.g.:
# [{"type": "thinking", "thinking": "..."}, {"type": "output", ...}]
# Without this branch the thinking text is silently dropped and the
# next turn fails with HTTP 400 ("thinking must be passed back").
# Refs #21944.
Comment on lines +3532 to +3538
for block in content:
if isinstance(block, dict) and block.get("type") == "thinking":
thinking_text = block.get("thinking") or block.get("text") or ""
thinking_text = thinking_text.strip()
if thinking_text and thinking_text not in reasoning_parts:
reasoning_parts.append(thinking_text)
if not reasoning_parts and isinstance(content, str) and content:
inline_patterns = (
r"<think>(.*?)</think>",
Expand Down Expand Up @@ -8057,6 +8070,11 @@ def _try_activate_fallback(self, reason: "FailoverReason | None" = None) -> bool
"Fallback to %s failed: provider not configured",
fb_provider)
return self._try_activate_fallback() # try next in chain
# Use the model the router actually resolved to (it may drop or
# adjust an incompatible override under provider: auto). Fall back
# to the configured value only if the router returned None.
if _resolved_fb_model:
fb_model = _resolved_fb_model
try:
from hermes_cli.model_normalize import normalize_model_for_provider

Expand All @@ -8067,6 +8085,26 @@ def _try_activate_fallback(self, reason: "FailoverReason | None" = None) -> bool
# Determine api_mode from provider / base URL / model
fb_api_mode = "chat_completions"
fb_base_url = str(fb_client.base_url)

# Self-selection guard: skip this fallback entry when its resolved
# base_url + model are identical to the currently active provider.
# Prevents custom-provider timeout loops where the fallback chain
# re-selects the same local shim endpoint that just failed (#22548).
_current_base_url = str(getattr(self, "base_url", "") or "").rstrip("/")
_fb_base_url_norm = fb_base_url.rstrip("/")
if (
_fb_base_url_norm
and _current_base_url
and _fb_base_url_norm == _current_base_url
and fb_model == (getattr(self, "model", "") or "")
):
logging.warning(
"Skipping fallback to %s/%s — same endpoint/model as current "
"provider; would loop. Trying next in chain.",
fb_provider, fb_model,
)
return self._try_activate_fallback(reason=reason)
Comment on lines +8089 to +8106

_fb_is_azure = self._is_azure_openai_url(fb_base_url)
if fb_provider == "openai-codex":
fb_api_mode = "codex_responses"
Expand Down
22 changes: 22 additions & 0 deletions tests/agent/test_error_classifier.py
Original file line number Diff line number Diff line change
Expand Up @@ -587,6 +587,28 @@ def test_timeout_error_builtin(self):
result = classify_api_error(e)
assert result.reason == FailoverReason.timeout

def test_runtime_error_cli_turn_timed_out_classifies_as_timeout(self):
# RuntimeError from a local claude-cli shim that wraps a subprocess
# timeout must classify as FailoverReason.timeout, not unknown, so
# the retry loop rebuilds the client instead of treating the turn as
# an empty model response (#22548).
e = RuntimeError("claude CLI turn timed out")
result = classify_api_error(e)
assert result.reason == FailoverReason.timeout
assert result.retryable is True

def test_runtime_error_request_timed_out_classifies_as_timeout(self):
e = RuntimeError("request timed out after 120s")
result = classify_api_error(e)
assert result.reason == FailoverReason.timeout
assert result.retryable is True

def test_runtime_error_deadline_exceeded_classifies_as_timeout(self):
e = RuntimeError("deadline exceeded")
result = classify_api_error(e)
assert result.reason == FailoverReason.timeout
assert result.retryable is True

# ── Error code classification ──

def test_error_code_resource_exhausted(self):
Expand Down
79 changes: 79 additions & 0 deletions tests/tools/test_session_search.py
Original file line number Diff line number Diff line change
Expand Up @@ -560,3 +560,82 @@ def _get_session(session_id):
assert entry["source"] == "api_server", (
f"source should be parent's 'api_server', got {entry['source']!r}"
)

def test_content_fetched_from_child_when_match_came_from_child(self):
"""Regression for #22150: when FTS5 hits a delegation/compression child
session, content for summarization must come from that child — not the
resolved parent — because the matched text only lives in the child.
"""
from unittest.mock import MagicMock, AsyncMock, patch as _patch
from tools.session_search_tool import session_search

mock_db = MagicMock()
mock_db.search_messages.return_value = [
{
"session_id": "child_sid",
"content": "discussion about headless-chatgpt setup",
"source": "cli",
"session_started": 1709400000,
"model": "gpt-4o-mini",
},
]

def _get_session(session_id):
if session_id == "child_sid":
return {
"id": "child_sid",
"parent_session_id": "parent_sid",
"source": "cli",
"started_at": 1709400000,
"model": "gpt-4o-mini",
}
if session_id == "parent_sid":
return {
"id": "parent_sid",
"parent_session_id": None,
"source": "cli",
"started_at": 1709300000,
"model": "gpt-4o-mini",
}
return None

mock_db.get_session.side_effect = _get_session

def _messages_for(sid):
if sid == "child_sid":
return [
{"role": "user", "content": "let's set up headless-chatgpt"},
{"role": "assistant", "content": "ok, here's how"},
]
if sid == "parent_sid":
return [
{"role": "user", "content": "delegated a task"},
{"role": "assistant", "content": "result delivered"},
]
return []

mock_db.get_messages_as_conversation.side_effect = _messages_for

with _patch(
"tools.session_search_tool.async_call_llm",
new_callable=AsyncMock,
side_effect=RuntimeError("no provider"),
):
result = json.loads(
session_search(query="headless-chatgpt", db=mock_db)
)

called_sids = [c.args[0] for c in mock_db.get_messages_as_conversation.call_args_list]
assert "child_sid" in called_sids, (
f"content retrieval must use the child session where the match lives, "
f"got calls for: {called_sids}"
)
assert "parent_sid" not in called_sids, (
f"content retrieval must NOT fetch parent's messages, got calls for: {called_sids}"
)

assert result["success"] is True
assert result["count"] == 1
entry = result["results"][0]
assert entry["session_id"] == "parent_sid"
assert "headless-chatgpt" in entry["summary"]
10 changes: 9 additions & 1 deletion tools/session_search_tool.py
Original file line number Diff line number Diff line change
Expand Up @@ -427,7 +427,14 @@ def _resolve_to_parent(session_id: str) -> str:
continue
if resolved_sid not in seen_sessions:
result = dict(result)
# Surface the resolved parent ID for display/dedup so callers
# see the canonical conversation, but remember the child where
# the FTS5 hit actually lives for content retrieval. Without
# this, get_messages_as_conversation() below would fetch the
# parent's messages — which often don't contain the matched
# text when the hit comes from a delegation/compression child.
result["session_id"] = resolved_sid
result["content_session_id"] = raw_sid
seen_sessions[resolved_sid] = result
if len(seen_sessions) >= limit:
break
Expand All @@ -436,7 +443,8 @@ def _resolve_to_parent(session_id: str) -> str:
tasks = []
for session_id, match_info in seen_sessions.items():
try:
messages = db.get_messages_as_conversation(session_id)
content_sid = match_info.get("content_session_id", session_id)
messages = db.get_messages_as_conversation(content_sid)
if not messages:
continue
session_meta = db.get_session(session_id) or {}
Expand Down
Loading