diff --git a/agent/agent_runtime_helpers.py b/agent/agent_runtime_helpers.py index 3017f00a10..db1b6b66ca 100644 --- a/agent/agent_runtime_helpers.py +++ b/agent/agent_runtime_helpers.py @@ -1,23 +1,7 @@ -"""Assorted AIAgent runtime helpers — moved out of run_agent.py for clarity. +"""Assorted AIAgent runtime helpers, moved out of run_agent.py. -Each function takes the parent ``AIAgent`` as its first argument -(``agent``) except for the static helpers (``sanitize_tool_call_arguments``, -``drop_thinking_only_and_merge_users``) which are stateless. AIAgent -keeps thin forwarders for backward compatibility. - -Methods covered: -* ``convert_to_trajectory_format`` — internal -> trajectory-file format -* ``sanitize_tool_call_arguments`` — repair corrupted JSON in tool_calls -* ``repair_message_sequence`` — enforce alternation invariants -* ``strip_think_blocks`` — remove inline reasoning from stored content -* ``recover_with_credential_pool`` — rotate pool entries on 429 -* ``try_recover_primary_transport`` — re-create OpenAI client after rate-limit -* ``drop_thinking_only_and_merge_users`` — Anthropic-style cleanup -* ``restore_primary_runtime`` — un-do fallback activation -* ``extract_reasoning`` — pull reasoning fields out of API responses -* ``dump_api_request_debug`` — write request body for post-mortem -* ``anthropic_prompt_cache_policy`` — compute cache_control breakpoints -* ``create_openai_client`` — build the per-agent OpenAI SDK client +Each function takes the parent ``AIAgent`` as ``agent`` except the stateless +helpers (``sanitize_tool_call_arguments``, ``drop_thinking_only_and_merge_users``). """ from __future__ import annotations @@ -54,11 +38,8 @@ from utils import base_url_host_matches, base_url_hostname, env_var_enabled, ato logger = logging.getLogger(__name__) -# Max consecutive successful credential-pool token refreshes of the SAME entry -# on a persistent auth failure before we give up and let the fallback chain -# activate. A single-entry OAuth pool can re-mint a fresh token indefinitely -# even when the upstream keeps rejecting it, so without this cap the retry loop -# spins forever and never reaches ``_try_activate_fallback``. See #26080. +# Cap consecutive same-entry OAuth token refreshes on a persistent auth failure; +# without it a single-entry pool re-mints forever and never reaches fallback (#26080). _MAX_AUTH_REFRESH_ATTEMPTS = 2 @@ -75,9 +56,8 @@ _TOOL_CALL_BLOCK_PATTERNS = tuple( for name in _TOOL_CALL_TAG_NAMES ) -# Named blocks — see strip_think_blocks step 1c for the -# full rationale (sentence-boundary lookbehind + tempered-dot body so a plain -# prose mention of "function" is never eaten). +# Named blocks; see strip_think_blocks step 1c for the +# boundary/tempered-dot rationale. _NAMED_FUNCTION_BLOCK_PATTERN = re.compile( r'(?:(?<=^)|(?<=[\n\r.!?:]))[ \t]*' r']*\bname\s*=[^>]*>' @@ -112,35 +92,13 @@ AGENT_RUNTIME_POST_HOOK_TOOL_NAMES = frozenset( ) -def agent_runtime_owns_post_tool_hook(agent: Any, function_name: str) -> bool: - """Return True when an agent-level tool path emits its own post hook.""" - if function_name in AGENT_RUNTIME_POST_HOOK_TOOL_NAMES: - return True - if getattr(agent, "_context_engine_tool_names", None) and function_name in agent._context_engine_tool_names: - return True - memory_manager = getattr(agent, "_memory_manager", None) - return bool(memory_manager and memory_manager.has_tool(function_name)) - - def convert_to_trajectory_format(agent, messages: List[Dict[str, Any]], user_query: str, completed: bool) -> List[Dict[str, Any]]: - """ - Convert internal message format to trajectory format for saving. - - Args: - messages (List[Dict]): Internal message history - user_query (str): Original user query - completed (bool): Whether the conversation completed successfully - - Returns: - List[Dict]: Messages in trajectory format - """ - # Normalize multimodal tool results — trajectories are text-only, so - # replace image-bearing tool messages with their text_summary to avoid - # embedding ~1MB base64 blobs into every saved trajectory. + """Convert internal message history to trajectory format for saving.""" + # Trajectories are text-only: swap image-bearing tool messages for their + # text_summary so ~1MB base64 blobs are not embedded. messages = [_trajectory_normalize_msg(m) for m in messages] trajectory = [] - # Add system message with tool definitions system_msg = ( "You are a function calling AI model. You are provided with function signatures within XML tags. " "You may call one or more functions to assist with the user query. If available tools are not relevant in assisting " @@ -160,25 +118,20 @@ def convert_to_trajectory_format(agent, messages: List[Dict[str, Any]], user_que "value": system_msg }) - # Add the actual user prompt (from the dataset) as the first human message trajectory.append({ "from": "human", "value": user_query }) - # Skip the first message (the user query) since we already added it above. - # Prefill messages are injected at API-call time only (not in the messages - # list), so no offset adjustment is needed here. + # Skip messages[0] (already added). Prefill is injected at API-call time + # only, so no offset adjustment is needed. i = 1 while i < len(messages): msg = messages[i] if msg["role"] == "assistant": - # Check if this message has tool calls if "tool_calls" in msg and msg["tool_calls"]: - # Format assistant message with tool calls - # Add tags around reasoning for trajectory storage content = "" # Prepend reasoning in tags if available (native thinking tokens) @@ -186,20 +139,16 @@ def convert_to_trajectory_format(agent, messages: List[Dict[str, Any]], user_que content = f"\n{msg['reasoning']}\n\n" if msg.get("content") and msg["content"].strip(): - # Convert any tags to tags - # (used when native thinking is disabled and model reasons via XML) + # -> (model reasons via XML when native thinking is off) content += convert_scratchpad_to_think(msg["content"]) + "\n" - # Add tool calls wrapped in XML tags for tool_call in msg["tool_calls"]: if not tool_call or not isinstance(tool_call, dict): continue - # Parse arguments - should always succeed since we validate during conversation - # but keep try-except as safety net + # Arguments were validated during conversation; try/except is a safety net try: arguments = json.loads(tool_call["function"]["arguments"]) if isinstance(tool_call["function"]["arguments"], str) else tool_call["function"]["arguments"] except json.JSONDecodeError: - # This shouldn't happen since we validate and retry during conversation, - # but if it does, log warning and use empty dict + # Should not happen (validated during the conversation); degrade to {} rather than abort. logger.warning("Unexpected invalid JSON in trajectory conversion: %s", tool_call['function']['arguments'][:100]) arguments = {} @@ -209,8 +158,7 @@ def convert_to_trajectory_format(agent, messages: List[Dict[str, Any]], user_que } content += f"\n{json.dumps(tool_call_json, ensure_ascii=False)}\n\n" - # Ensure every gpt turn has a block (empty if no reasoning) - # so the format is consistent for training data + # Every gpt turn gets a block (empty if none) for a consistent training format if "" not in content: content = "\n\n" + content @@ -219,15 +167,13 @@ def convert_to_trajectory_format(agent, messages: List[Dict[str, Any]], user_que "value": content.rstrip() }) - # Collect all subsequent tool responses tool_responses = [] j = i + 1 while j < len(messages) and messages[j]["role"] == "tool": tool_msg = messages[j] - # Format tool response with XML tags tool_response = "\n" - # Try to parse tool content as JSON if it looks like JSON + # Pretty-print tool content if it looks like JSON tool_content = tool_msg["content"] try: if tool_content.strip().startswith(("{", "[")): @@ -250,7 +196,6 @@ def convert_to_trajectory_format(agent, messages: List[Dict[str, Any]], user_que tool_responses.append(tool_response) j += 1 - # Add all tool responses as a single message if tool_responses: trajectory.append({ "from": "tool", @@ -259,20 +204,17 @@ def convert_to_trajectory_format(agent, messages: List[Dict[str, Any]], user_que i = j - 1 # Skip the tool messages we just processed else: - # Regular assistant message without tool calls - # Add tags around reasoning for trajectory storage content = "" # Prepend reasoning in tags if available (native thinking tokens) if msg.get("reasoning") and msg["reasoning"].strip(): content = f"\n{msg['reasoning']}\n\n" - # Convert any tags to tags - # (used when native thinking is disabled and model reasons via XML) + # -> (model reasons via XML when native thinking is off) raw_content = msg["content"] or "" content += convert_scratchpad_to_think(raw_content) - # Ensure every gpt turn has a block (empty if no reasoning) + # Every gpt turn gets a block (empty if none) for a consistent training format if "" not in content: content = "\n\n" + content @@ -302,25 +244,13 @@ def sanitize_tool_call_arguments( ) -> int: """Repair corrupted assistant tool-call argument JSON in-place. - ``cursor`` (optional) is a caller-owned dict used to skip re-validating - messages already validated on a previous call. It stores, under - ``"prefix"``, the exact message *objects* (strong references) validated - last time, in order. On the next call, the longest contiguous prefix of - ``messages`` whose objects are ``is``-identical to the stored prefix is - skipped; scanning starts at the first divergence (conservative: any - reordering, truncation, compression rewrite, or mid-list insertion breaks - identity at that index and everything from there is re-scanned). - - Safety argument for skipping: a message in the matched prefix was fully - scanned before — every tool_call argument was either already valid JSON - or was rewritten to ``"{}"`` (valid). The only code paths that mutate - ``function["arguments"]`` on live history dicts between calls are the - surrogate / non-ASCII sanitizers, which substitute characters *inside* - JSON string values and cannot invalidate JSON syntax. Compression, - repair, undo, and steer paths replace or reorder message dicts, which - breaks the identity match and forces a re-scan. Holding strong - references (the objects themselves, not ``id()``s) makes address reuse - aliasing (#50372-style) impossible. + ``cursor`` (optional caller-owned dict) stores under ``"prefix"`` strong + references to the message objects validated last call; the longest + ``is``-identical prefix is skipped on the next call. Skipping is safe + because only the surrogate/non-ASCII sanitizers mutate arguments on live + dicts (inside JSON string values), and every other path replaces or + reorders dicts, breaking identity. Strong refs (not ``id()``) rule out + address-reuse aliasing (#50372). """ log = logger or logging.getLogger(__name__) if not isinstance(messages, list): @@ -387,22 +317,12 @@ def sanitize_tool_call_arguments( try: json.loads(arguments) except json.JSONDecodeError: - # Use the canonical ``call_id || id`` precedence so both the - # scan for an existing tool result and any inserted stub key - # on the same id the rest of the pipeline uses. Keying on bare - # ``id`` here would fail to find a result built with ``call_id`` - # (Codex Responses format) and insert a duplicate stub that - # itself becomes an orphan (#58168). + # Use canonical ``call_id || id`` precedence so scan and stub share the id + # the pipeline uses; bare ``id`` misses Codex call_id results and orphans a stub (#58168). tool_call_id = _ra().AIAgent._get_tool_call_id_static(tool_call) or None function_name = function.get("name", "?") - # Log the FULL original argument string (bounded), not an - # 80-char preview: this branch is about to overwrite the - # only copy of these bytes in the transcript with "{}", and - # for a truncated write_file/patch call the destroyed - # arguments contain real user content (#80498 — streamed - # file content survived only as a log preview). A corrupted - # call is rare, so the oversized WARNING is a fair price for - # making the data recoverable from agent.log. + # Log the FULL (bounded) argument string: we are about to overwrite the only + # copy, which may hold real user content from a truncated write_file/patch (#80498). preview = arguments[:_FULL_ARGS_LOG_BOUND] log.warning( "Corrupted tool_call arguments repaired before request " @@ -448,45 +368,28 @@ def sanitize_tool_call_arguments( message_index += 1 if cursor is not None: - # Strong references to the exact objects validated this call, in - # order. Any future divergence (compression, undo, repair, steer) - # breaks identity at the divergent index and re-scans from there. + # Strong refs to the objects validated this call; any divergence + # (compression, undo, repair, steer) forces a re-scan from that index. cursor["prefix"] = messages[:] return repaired -# Session-scoped in-flight registry backing note_turn_start's cross-agent -# check. The per-agent marker catches a second turn on the SAME AIAgent -# object, but the gateway caches agents per *routing key* (``_agent_cache`` -# in gateway/run.py) while the durable transcript is keyed by *session_id* — -# and the key→id mapping is many-to-one (``switch_session``: /resume from a -# second chat/topic, CLI-continuity rebinding, async-delegation pinning, -# topic-binding tip-walks). Two routing keys mapped to one session_id run -# concurrent turns on two different agent objects, which per-agent state can -# never see (#64934). Keyed by session_id so that route produces the same -# named warning. Process-local by design — same visibility scope as the -# per-agent marker it extends. +# Session-scoped in-flight registry for note_turn_start. The gateway caches agents +# per routing key while the transcript is keyed by session_id (many-to-one), so two +# agent objects can run concurrent turns on one session unseen by per-agent state (#64934). _INFLIGHT_TURNS_BY_SESSION: Dict[str, Tuple[str, float]] = {} _INFLIGHT_TURNS_LOCK = threading.Lock() def note_turn_start(agent, turn_id: str): - """Tripwire: detect a turn starting while a previous turn of the same - agent — or of the same underlying *session* on a different agent object — - has not completed its turn-end persist. + """Tripwire: warn when a turn starts while a previous turn of the same agent + or the same session (on another agent object) has not finished its persist. - Two turns interleaving on one session corrupt the durable transcript: - their flushes race (user rows can persist out of arrival order), a row - can be swallowed by the identity-marker dedup over shared history dicts, - and the second turn runs on a history base that never saw the first - turn's exchange. This helper does NOT prevent any of that — it names the - occurrence, with both turn ids, so the dispatch route that let the - second turn through the busy guard can be identified from logs. - - Returns the previous in-flight turn_id when an overlap is detected, - else None. Takes ownership of the in-flight slot either way, so a turn - that crashed before its persist produces at most one warning.""" + Does not prevent the overlap; it names it with both turn ids so the dispatch + route that bypassed the busy guard can be found in logs. Returns the previous + in-flight turn_id on overlap, else None; takes ownership of the slot either way. + """ prev = getattr(agent, "_inflight_turn_id", None) prev_started = getattr(agent, "_inflight_turn_started", 0.0) agent._inflight_turn_id = turn_id @@ -504,23 +407,18 @@ def note_turn_start(agent, turn_id: str): ) overlap = prev - # Cross-agent leg: same session_id in flight under a different agent - # object means two routing keys resolve to one durable session — the - # busy guard (keyed by routing key) cannot see this overlap at all. - # Persist-disabled agents (background-review forks) deliberately share - # the live parent's session_id for prompt-cache warmth but can never - # write to the transcript — they must not register here (would warn a - # false overlap against the parent's real turn) nor pop the parent's - # slot at their persist (note_turn_persisted skips them symmetrically). + # Cross-agent leg: same session_id in flight under another agent object + # (busy guard is keyed by routing key and cannot see it). Persist-disabled + # forks share the parent's session_id but never write, so they must not + # register or pop here (note_turn_persisted skips them symmetrically). session_id = getattr(agent, "session_id", None) if session_id and not getattr(agent, "_persist_disabled", False): now = time.time() with _INFLIGHT_TURNS_LOCK: entry = _INFLIGHT_TURNS_BY_SESSION.get(session_id) _INFLIGHT_TURNS_BY_SESSION[session_id] = (turn_id, now) - # Stamp the session id this turn registered under: compression can - # rotate agent.session_id mid-turn, and the persist-time clear must - # pop the slot the turn actually holds, not the rotated id. + # Record the session id registered under: compression can rotate + # agent.session_id mid-turn and persist must pop the slot actually held. agent._inflight_turn_session_id = session_id if entry and entry[0] not in (turn_id, prev): logger.warning( @@ -540,15 +438,12 @@ def note_turn_start(agent, turn_id: str): def note_turn_persisted(agent): """Clear the in-flight marker at turn-end persist (see note_turn_start). - Called from the single persist funnel; unconditional by design — when two - turns genuinely overlap, the first persist clears the second turn's slot - and the tripwire under-reports instead of double-reporting. A diagnostic - must never be noisier than the defect it hunts.""" + Unconditional by design: on a real overlap the first persist clears the + second slot and the tripwire under-reports rather than double-reports. + """ agent._inflight_turn_id = None - # Symmetric with note_turn_start's cross-agent leg: persist-disabled - # forks never registered a session slot, and their persist funnel still - # runs — popping here would steal the live parent turn's slot and make - # the tripwire under-report the real overlap it exists to catch. + # Persist-disabled forks never registered a slot; popping here would + # steal the live parent turn's slot (symmetric with note_turn_start). if not getattr(agent, "_persist_disabled", False): session_id = getattr(agent, "_inflight_turn_session_id", None) or getattr( agent, "session_id", None @@ -562,81 +457,27 @@ def note_turn_persisted(agent): def repair_message_sequence(agent, messages: List[Dict]) -> int: """Collapse malformed role-alternation left in the live history. - Providers (OpenAI, OpenRouter, Anthropic) expect strict alternation: - after the system message, user/tool alternates with assistant, with - no two consecutive user messages and no tool-result that doesn't - follow an assistant-with-tool_calls. Violations cause silent empty - responses on most providers, which triggers the empty-retry loop. + Providers require strict alternation after the system message; violations + cause silent empty responses or HTTP 400s. Runs right before the API call as + a defensive belt for host-fed, resumed, or replayed histories. - This runs right before the API call as a defensive belt — by the - time it fires, the scaffolding strip should already have prevented - most shapes, but external callers (gateway multi-queue replay, - session resume, cron, explicit conversation_history passed in by - host code) can feed in already-broken histories. + Passes: 0. merge consecutive assistant turns (union tool_calls, concat + content; codex interim turns exempt; #29148, #49147); 1. drop stray ``tool`` + results with no preceding matching tool_call; 2. prune tool_calls not + answered in the immediately following tool run, dropping the turn if left + empty (codex interims exempt); 3. merge consecutive user messages. + A user turn directly after an assistant turn is valid and left alone. - Repairs applied: - 0. Consecutive ``assistant`` messages with no intervening - ``tool``/``user`` turn — merged into a single assistant turn - (union of ``tool_calls``, concatenated ``content``). Strict - OpenAI-compatible providers (DeepSeek v4, Moonshot/Kimi) reject - a history where an ``assistant`` message carrying ``tool_calls`` - is immediately followed by another ``assistant`` message instead - of its ``tool`` results — HTTP 400 "An assistant message with - 'tool_calls' must be followed by tool messages…". The split - shape is produced by recovery/continuation paths that append an - interim assistant turn (thinking-prefill, codex - incomplete-continuation) or by host-fed / legacy-persisted / - resumed histories. Refs #29148, #49147. - 1. Stray ``tool`` messages whose ``tool_call_id`` doesn't match - any preceding assistant tool_call — dropped. - 2. ``tool_calls`` on an assistant message that no immediately - following ``tool`` result answers are pruned — and the turn is - dropped entirely if that leaves it payload-empty (an empty - non-final assistant message is itself a 400 on most providers). - 3. Consecutive ``user`` messages — merged with newline separator - so no user input is lost. - - Deliberately does NOT rewind orphan ``assistant(tool_calls)+tool`` - pairs that precede a user message — that pattern IS valid when the - previous turn completed normally and the user jumped in to redirect - before the model got a continuation turn (the ongoing dialog - pattern). The empty-response scaffolding stripper handles the - genuinely-broken variant via its flag-gated rewind. - - Pass 2 (prune unanswered ``tool_calls``) answers the complement of - Pass 1: Pass 1 removes the stray result, Pass 2 removes the orphaned - call it was displaced from. Context compression can move a tool - result past a user turn; without this pass the declaring assistant - message would keep replaying an unanswered ``tool_call`` and strict - providers (DeepSeek v4) reject that with HTTP 400 "An assistant - message with 'tool_calls' must be followed by tool messages - responding to each 'tool_call_id'". A call counts as answered when - the run of ``tool`` messages immediately following its assistant - message contains a result keyed to ANY of the call's ids (``id`` or - ``call_id`` — the same superset rule Pass 1 registers). Codex - interim turns are exempt, as in Pass 0. - - Returns the number of repairs made (for logging/telemetry). + Returns the number of repairs made. """ if not messages: return 0 repairs = 0 - # Pass 0: merge consecutive assistant messages. Runs BEFORE Pass 1 so - # the merged turn's union of tool_call ids is known when Pass 1 - # validates which tool-result messages are orphans. Two assistant - # messages are only adjacent here when nothing (no tool result, no - # user turn) separates them — an intervening ``tool`` message means - # two distinct, valid tool-call rounds that must NOT be merged. - # - # Codex Responses interim turns are exempt: the codex_responses - # api_mode legitimately keeps multiple consecutive incomplete - # assistant turns in history, each carrying its own encrypted - # continuation state (codex_reasoning_items / codex_message_items) - # that must be replayed verbatim. Collapsing them corrupts the - # Responses replay chain (the duplicate-detection logic at - # conversation_loop.py already de-dups identical codex interims). + # Pass 0: merge consecutive assistant messages, BEFORE Pass 1 so the merged + # tool_call-id union is known. Codex Responses interim turns are exempt: + # each carries its own continuation state that must be replayed verbatim. def _is_codex_interim(m: Dict) -> bool: return bool( m.get("codex_reasoning_items") @@ -662,12 +503,8 @@ def repair_message_sequence(agent, messages: List[Dict]) -> int: and not _is_codex_interim(collapsed[-1]) ): prev = collapsed[-1] - # Verification candidate collapsing: when the earlier assistant - # message is a provisional candidate (finish_reason = - # verification_required / verify_hook_continue), the later - # response supersedes it for model replay — replace rather than - # union. Both remain durable in state.db; this only affects the - # in-memory sequence sent to the model. (#65919 §7) + # A provisional verification candidate (finish_reason = + # verification_required / verify_hook_continue) is superseded, not unioned (#65919 §7). if _is_verification_candidate(prev): collapsed[-1] = msg repairs += 1 @@ -680,24 +517,10 @@ def repair_message_sequence(agent, messages: List[Dict]) -> int: elif prev_calls: prev["tool_calls"] = prev_calls else: - # Neither turn carries tool calls, but the surviving turn may - # still carry a stale ``tool_calls: []`` from the earlier - # message. An empty array is semantically "no tool calls", - # yet strict OpenAI-compatible providers (DeepSeek v4, - # Moonshot/Kimi) reject it with HTTP 400 ("Invalid - # 'messages[N].tool_calls': empty array..."). Drop the key - # HERE, at the source: ``sanitize_api_messages`` only fixes - # the per-call wire copy, so a ``[]`` left on the repaired - # turn survives in the live/persisted trajectory returned to - # callers (gateway/WebUI transcripts, session resume, - # subagents, cron) and is replayed on the next turn — which - # is how #58755 kept reproducing after the chokepoint fix - # (#77921). Popping is non-destructive: an empty array - # carries no information. + # Drop a stale ``tool_calls: []`` at the source: strict providers + # (DeepSeek v4, Kimi) 400 on it and it persists into replayed history (#58755, #77921). prev.pop("tool_calls", None) - # Concatenate plain-text content; leave multimodal (list) - # content on either side alone to avoid mangling attachment - # blocks — fall back to keeping the existing content. + # Concatenate plain-text content only; leave multimodal (list) content alone. prev_content = prev.get("content") new_content = msg.get("content") content_rewritten = False @@ -706,53 +529,26 @@ def repair_message_sequence(agent, messages: List[Dict]) -> int: p for p in (prev_content.strip(), new_content.strip()) if p ) prev["content"] = joined - # A falsy ``new_content`` (e.g. "") strips to nothing and - # ``joined`` collapses back to ``prev_content`` unchanged -- - # that must NOT count as a rewrite (wz-heng, #78063 review). + # A falsy new_content leaves ``joined`` == prev_content; that is not a rewrite (#78063). content_rewritten = joined != prev_content elif not prev_content and new_content is not None: prev["content"] = new_content content_rewritten = new_content != prev_content - # Carry reasoning_content from the later turn only if the - # earlier turn lacks it (strict thinking providers require a - # reasoning_content on the merged tool-call turn; the first - # non-empty one suffices). + # Carry reasoning_content from the later turn only if the earlier lacks it + # (strict thinking providers need one on the merged tool-call turn). if not prev.get("reasoning_content") and msg.get("reasoning_content"): prev["reasoning_content"] = msg["reasoning_content"] - # ``prev`` may carry an ``api_content`` sidecar (the exact bytes - # previously sent to the API, e.g. a sanitize-divergence stamp — - # see ``_flush_messages_to_session_db``) from BEFORE this merge. - # The sidecar takes priority over ``content`` at API-build time - # (``conversation_loop``'s ``api_messages`` build substitutes it - # back in for role ``assistant``), so leaving it in place while - # ``prev["content"]`` changes would silently replay the pre-merge - # bytes and discard everything this merge just concatenated on — - # the same stale-field-survives-the-merge shape as the - # ``tool_calls`` gap above, just for a different field. Only drop - # it when the merge actually changed the resulting value (e.g. - # the later turn's content is ``None``, or either side is - # multimodal/list — both branches skip the reassignment and - # ``prev["content"]`` is untouched; a falsy ``new_content`` that - # strips to nothing also leaves ``joined`` equal to the original - # ``prev_content``): in those cases the sidecar is still the - # exact bytes previously sent for the UNCHANGED content, and - # dropping it would break the prompt-cache replay invariant for - # no reason (wz-heng, #78063 review). + # A stale ``api_content`` sidecar overrides ``content`` at API-build time and + # would replay pre-merge bytes; drop it only when content actually changed (#78063). if content_rewritten: drop_stale_api_content(prev) repairs += 1 continue collapsed.append(msg) - # Pass 1: drop stray tool messages that don't follow a known assistant - # tool call. A Responses call can have several equivalent spellings - # (call_id, id, response_item_id, or a composite ``call|item`` id), so - # consume the whole alias group when one spelling is matched. Otherwise a - # duplicate result keyed on the sibling alias would survive and be replayed - # to strict providers (#66974). Alias expansion lives in - # ``agent.message_sanitization.tool_call_id_variants`` / - # ``tool_result_id_variants`` (single policy owner) — which also handles - # SDK tool_call objects, preserving the #91768 dict-or-object tolerance. + # Pass 1: drop stray tool messages not following a known assistant tool call. + # Consume the whole alias group (call_id/id/response_item_id/composite) so a + # duplicate keyed on a sibling alias is not replayed to strict providers (#66974, #91768). known_tool_ids: Dict[str, int] = {} matched_tool_groups: set = set() next_tool_group = 0 @@ -785,10 +581,8 @@ def repair_message_sequence(agent, messages: List[Dict]) -> int: if not result_variants: filtered.append(msg) elif candidate_groups: - # Consume the whole alias group so a SECOND result replaying - # any sibling spelling falls into the drop branch below — - # strict providers reject duplicate tool_call_ids with HTTP - # 400 (#58327, #66974). Credit: #55436. + # Consume the whole alias group so a second result on any sibling + # spelling is dropped; strict providers 400 on duplicates (#58327, #66974, #55436). group_id = min(candidate_groups) filtered.append(msg) matched_tool_groups.add(group_id) @@ -796,39 +590,14 @@ def repair_message_sequence(agent, messages: List[Dict]) -> int: repairs += 1 else: if role == "user": - # A user turn closes the tool-result run; subsequent - # tool messages without a fresh assistant tool_call - # are orphans. + # A user turn closes the tool-result run; later tool messages are orphans. known_tool_ids = {} matched_tool_groups = set() filtered.append(msg) - # Pass 2: prune tool_calls that were never answered positionally. - # - # Pass 1 dropped the stray/displaced tool RESULT — but a tool_call - # whose result was displaced far beyond the following turn (context - # compression can move it past a user turn) leaves its declaring - # assistant message carrying an UNANSWERED tool_call, and strict - # OpenAI-compatible providers (DeepSeek v4) reject the payload with - # HTTP 400 "An assistant message with 'tool_calls' must be followed - # by tool messages responding to each 'tool_call_id' (insufficient - # tool messages following tool_calls message)". The per-call - # sanitizer's stub pass is keyed on GLOBAL id presence, which a - # displaced-but-present result masks (see sanitize_api_messages) — - # so the durable history must not keep replaying the poisoned turn - # either. Enforce the positional invariant here: a tool_call is only - # legitimate when a result for ANY of its id variants (``id`` / - # ``call_id`` / ``response_item_id`` / composite bridge — the same - # unified alias policy as Pass 1, via ``tool_call_id_variants`` / - # ``tool_result_id_variants``) appears in the run of tool messages - # IMMEDIATELY following the declaring assistant message — before any - # user turn or further assistant turn. Unanswered calls are pruned; - # if the message then carries no other payload (no content, - # reasoning, codex items), the whole turn is dropped — an empty - # non-final assistant message is itself rejected by providers. - # Codex interim turns are exempt, as in Pass 0: their calls are - # replayed through the Responses-items chain, not the tool-result - # run. + # Pass 2: prune tool_calls never answered in the IMMEDIATELY following tool run + # (any id variant, same alias policy as Pass 1); a displaced result masks the per-call + # stub pass and strict providers (DeepSeek v4) 400. Drop payload-empty turns; codex interims exempt. pruned: List[Dict] = [] i = 0 n = len(filtered) @@ -867,9 +636,7 @@ def repair_message_sequence(agent, messages: List[Dict]) -> int: if not kept_calls and not _msg_has_payload( {k: v for k, v in msg.items() if k != "tool_calls"} ): - # The pruned call(s) were the message's only payload — - # dropping the whole turn beats sending an empty - # assistant message (which most providers 400). + # Pruned calls were the only payload; drop the turn (empty assistant messages 400). i += 1 continue if kept_calls: @@ -879,8 +646,7 @@ def repair_message_sequence(agent, messages: List[Dict]) -> int: pruned.append(msg) i += 1 - # Pass 3: merge consecutive user messages. Preserves all user input - # so nothing the user typed is lost. + # Pass 3: merge consecutive user messages (no user input lost). merged: List[Dict] = [] for msg in pruned: if ( @@ -891,12 +657,8 @@ def repair_message_sequence(agent, messages: List[Dict]) -> int: and merged[-1].get("role") == "user" ): prev = merged[-1] - # A summary carrier followed by a new user row is a deliberate - # durable shape after retry/rewind. Do not absorb the fresh ask - # into the already-persisted carrier: mutating that dict can make - # the only in-memory copy diverge from its durable row. Provider - # sanitizers merge copies later when strict alternation requires - # it, without rewriting either durable message. + # A summary carrier followed by a new user row is a deliberate durable shape + # after retry/rewind; never mutate the persisted carrier (sanitizers merge copies later). from agent.context_compressor import split_user_originated_turn handoff, _ = split_user_originated_turn(prev) @@ -906,49 +668,32 @@ def repair_message_sequence(agent, messages: List[Dict]) -> int: prev_content = prev.get("content", "") new_content = msg.get("content", "") - # Only merge plain-text content; leave multimodal (list) - # content alone — collapsing image/audio blocks risks - # mangling the attachment structure. + # Only merge plain-text content; leave multimodal (list) content alone. if isinstance(prev_content, str) and isinstance(new_content, str): prev["content"] = ( (prev_content + "\n\n" + new_content) if prev_content and new_content else (prev_content or new_content) ) - # Merged content invalidates the api_content sidecar (exact - # bytes previously sent for the pre-merge message) — drop it - # so replay can't substitute stale bytes. + # Merged content invalidates the api_content sidecar; drop it so replay cannot use stale bytes. drop_stale_api_content(prev) repairs += 1 continue merged.append(msg) if repairs > 0: - # Rewrite in place so downstream paths (persistence, return - # value, session DB flush) see the repaired sequence. + # Rewrite in place so persistence/return value/DB flush see the repaired sequence. messages[:] = merged return repairs def repair_message_sequence_with_cursor(agent, messages: List[Dict]) -> int: - """Run :func:`repair_message_sequence` and keep the SessionDB flush - cursor consistent with the compacted list (#44837). + """Run :func:`repair_message_sequence` and keep ``_last_flushed_db_idx`` consistent (#44837). - ``repair_message_sequence`` merges/drops messages in place, shrinking - the list. ``_last_flushed_db_idx`` (the DB-write cursor) indexes into - that list, so after compaction it can point past the new end — the - turn-end flush would then skip the assistant/tool chain entirely — or - past unflushed messages shifted to lower indexes. - - Repair preserves object identity for surviving messages, so counting - the survivors from the previously-flushed prefix gives the exact new - cursor even when messages are dropped/merged at indexes *before* the - cursor — a plain ``min()`` clamp would silently skip that many - unflushed rows. Falls back to the clamp when no prefix snapshot is - available. - - Returns the number of repairs made (same as ``repair_message_sequence``). + Repair shrinks the list in place; counting survivors from the flushed prefix + (identity-preserved) gives the exact new cursor, whereas a ``min()`` clamp + would skip unflushed rows. Falls back to the clamp without a snapshot. """ pre_repair_flushed_ids = None flush_cursor = getattr(agent, "_last_flushed_db_idx", None) @@ -974,46 +719,18 @@ def repair_message_sequence_with_cursor(agent, messages: List[Dict]) -> int: def strip_think_blocks(agent, content: str) -> str: """Remove reasoning/thinking blocks from content, returning only visible text. - Handles four cases: - 1. Closed tag pairs (`` … ``) — the common path when - the provider emits complete reasoning blocks. - 2. Unterminated open tag at a block boundary (start of text or - after a newline) — e.g. MiniMax M2.7 / NIM endpoints where the - closing tag is dropped. Everything from the open tag to end - of string is stripped. The block-boundary check mirrors - ``gateway/stream_consumer.py``'s filter so models that mention - `` `` in prose aren't over-stripped. - 3. Stray orphan open/close tags that slip through. - 4. Tag variants: `` ``, ````, ````, - ````, ```` (Gemma 4), all - case-insensitive. - - Additionally strips standalone tool-call XML blocks that some open - models (notably Gemma variants on OpenRouter) emit inside assistant - content instead of via the structured ``tool_calls`` field: - * ``…`` - * ``…`` - * ``…`` - * ``…`` - * ``…`` - * ``…`` (Gemma style) - Ported from openclaw/openclaw#67318. The ```` variant is - boundary-gated (only strips when the tag sits at start-of-line or - after punctuation and carries a ``name="..."`` attribute) so prose - mentions like "Use in JavaScript" are preserved. + Strips closed tag pairs, unterminated open tags at a block boundary (mirrors + ``gateway/stream_consumer.py``), stray orphan tags, and all case-insensitive + variants (think/thinking/reasoning/REASONING_SCRATCHPAD/thought). Also strips + standalone tool-call XML blocks some open models emit in content (ported from + openclaw/openclaw#67318); the ```` variant is boundary- and + ``name=``-gated so prose mentions survive. """ if not content: return "" - # Coerce non-string content to text before any regex runs. Providers - # that return assistant ``content`` as a list of blocks (Anthropic via - # OpenRouter emits ``[{"type":"text",...}, {"type":"thinking",...}]``) or - # as a dict flow into this shared helper from several callers — most - # notably ``_interim_assistant_visible_text`` reading a *stored* history - # message whose content was persisted as a list. A raw list/dict reaching - # ``re.sub`` below raises ``TypeError: expected string or bytes-like - # object, got 'list'``, which the outer conversation loop swallows and - # retries forever (observed as an infinite "preparing terminal…" loop on - # Anthropic models via OpenRouter). Flatten here so every caller is safe. + # Flatten list/dict content (e.g. Anthropic-via-OpenRouter block lists from + # stored history) before regex: a raw list hits re.sub, raises TypeError, + # and the conversation loop retries forever. if not isinstance(content, str): if isinstance(content, list): _parts: list[str] = [] @@ -1022,9 +739,7 @@ def strip_think_blocks(agent, content: str) -> str: _parts.append(_part) elif isinstance(_part, dict): _ptype = str(_part.get("type") or "").strip().lower() - # Drop reasoning/thinking blocks outright — this function's - # whole job is to strip them, and their text lives under - # different keys ("thinking", "reasoning") per provider. + # Drop thinking/reasoning blocks outright; their text key varies per provider. if _ptype in {"thinking", "reasoning", "redacted_thinking"}: continue _text = _part.get("text") @@ -1037,33 +752,23 @@ def strip_think_blocks(agent, content: str) -> str: content = str(content) if not content: return "" - # 1. Closed tag pairs — case-insensitive for all variants so - # mixed-case tags (, ) don't slip through to - # the unterminated-tag pass and take trailing content with them. + # 1. Closed tag pairs, case-insensitive so mixed-case tags do not fall + # through to the unterminated pass and eat trailing content. for _pattern in _REASONING_BLOCK_PATTERNS: content = _pattern.sub('', content) - # 1b. Tool-call XML blocks (openclaw/openclaw#67318). Handle the - # generic tag names first — they have no attribute gating since - # a literal in prose is already vanishingly rare. + # 1b. Tool-call XML blocks (openclaw/openclaw#67318); generic tags need no attribute gating. for _pattern in _TOOL_CALL_BLOCK_PATTERNS: content = _pattern.sub('', content) - # 1c. ... — Gemma-style standalone - # tool call. Only strip when the tag sits at a block boundary - # (start of text, after a newline, or after sentence-ending - # punctuation) AND carries a name="..." attribute. This keeps - # prose mentions like "Use to declare" safe. + # 1c. Gemma-style block: strip only at a block boundary + # AND with a name attribute so prose mentions of survive. content = _NAMED_FUNCTION_BLOCK_PATTERN.sub('', content) - # 2. Unterminated reasoning block — open tag at a block boundary - # (start of text, or after a newline) with no matching close. - # Strip from the tag to end of string. Fixes #8878 / #9568 - # (MiniMax M2.7 leaking raw reasoning into assistant content). + # 2. Unterminated reasoning block at a block boundary: strip to end of + # string (#8878, #9568: MiniMax M2.7 leaking raw reasoning). content = _UNTERMINATED_REASONING_BLOCK_PATTERN.sub('', content) # 3. Stray orphan open/close tags that slipped through. content = _ORPHAN_REASONING_TAG_PATTERN.sub('', content) - # 3b. Stray tool-call closers. (We do NOT strip bare or - # unterminated because a truncated tail - # during streaming may still be valuable to the user; matches - # OpenClaw's intentional asymmetry.) + # 3b. Stray tool-call closers only; bare/unterminated is kept since a + # truncated streaming tail may still be valuable (matches OpenClaw asymmetry). content = _STRAY_TOOL_CALL_CLOSER_PATTERN.sub('', content) return content @@ -1072,10 +777,8 @@ def strip_think_blocks(agent, content: str) -> str: def sync_credential_pool_entry_id(agent) -> None: """Rebind ``agent._credential_pool_entry_id`` from the current pool + key. - OAuth refreshes can replace the runtime token before a failed request is - recovered, so the mutable API-key value alone cannot reliably attribute - the failure to its source entry. This resolves the stable pool-entry ID - for the agent's current ``api_key`` and clears it when no pool is bound. + OAuth refreshes can replace the token before recovery runs, so the key + alone cannot attribute a failure; the stable entry ID can. Cleared when no pool is bound. """ pool = getattr(agent, "_credential_pool", None) try: @@ -1099,49 +802,26 @@ def recover_with_credential_pool( ) -> tuple[bool, bool]: """Attempt credential recovery via pool rotation. - Returns (recovered, has_retried_429). - On rate limits: first occurrence retries same credential (sets flag True). - second consecutive failure rotates to next credential. - On billing exhaustion: immediately rotates. - On auth failures: attempts token refresh before rotating. - - `classified_reason` lets the recovery path honor the structured error - classifier instead of relying only on raw HTTP codes. This matters for - providers that surface billing/rate-limit/auth conditions under a - different status code, such as Anthropic returning HTTP 400 for - "out of extra usage". - - `billing_unverified` marks a billing verdict that rests on an ambiguous - body (``ClassifiedError.billing_unverified``, #82154): the pool persists - it as ``billing_unverified`` so the exhausted entry gets a short cooldown - instead of the one-hour billing bench — the same 400 can be a - content-filter rejection that leaves the credential healthy. + Returns (recovered, has_retried_429). Rate limits: retry once, then rotate. + Billing exhaustion: rotate immediately. Auth failures: refresh before rotating. + ``classified_reason`` honors the structured classifier over raw HTTP codes + (e.g. Anthropic 400 for "out of extra usage"). ``billing_unverified`` (#82154) + persists an ambiguous billing verdict so the entry gets a short cooldown, not + the one-hour bench. """ pool = agent._credential_pool if pool is None: return False, has_retried_429 - # Defensive guard: if a fallback provider is active and its provider name - # doesn't match the pool's provider, the pool belongs to the PRIMARY - # provider. Mutating it based on fallback errors would corrupt the - # primary's credential state (see #33088) and, via _swap_credential, - # overwrite the agent's base_url back to the primary's endpoint — every - # subsequent request then goes to the wrong host and 404s (see #33163). - # The pool should only act when the agent is still on the same provider - # that seeded the pool. + # The pool belongs to the PRIMARY provider: acting on fallback errors would + # corrupt its state (#33088) and reset base_url to the primary endpoint (#33163). current_provider = (getattr(agent, "provider", "") or "").strip().lower() pool_provider = (getattr(pool, "provider", "") or "").strip().lower() - # Guard: skip credential pool recovery when the pool is scoped to a - # different provider than the agent. Only guard when the pool has a - # known provider — an empty pool provider means "unscoped" (applies to - # any provider). An empty agent provider is treated as a mismatch - # because swapping the pool's credentials would set base_url/api_key - # without fixing the empty provider field, leaving the agent in a - # corrupted state (provider="" model=""). + # Skip recovery when the pool is scoped to another provider. Empty pool provider + # means unscoped; empty agent provider is a mismatch (swap would leave provider="" model=""). if pool_provider: - # Use the same fail-closed boundary predicate as runtime binding. This - # recognizes configured named-custom aliases, validates endpoints even - # for exact custom:* identities, and preserves fallback isolation. + # Same fail-closed boundary predicate as runtime binding (named-custom + # aliases, endpoint validation, fallback isolation). if not credential_pool_matches_provider( pool, current_provider, @@ -1154,17 +834,9 @@ def recover_with_credential_pool( ) return False, has_retried_429 - # Attribute the failure to the API key the agent actually dispatched the - # request with, not to pool.current(). The current() pointer is shared, - # mutable state — round-robin select() advances it on every call, and - # concurrent turns or a second process (gateway/dashboard) reloading the - # pool reset it to None — so by the time recovery runs it routinely points - # at a DIFFERENT, healthy entry. Marking that entry exhausted copies this - # request's error/reset time onto it and can take the whole pool offline - # from a single rate-limited key (#43747). ``_swap_credential`` keeps - # ``agent.api_key`` in sync with the entry in use, so it identifies the - # failing entry exactly; fall back to current()'s key only when the agent - # carries no key at all. + # Attribute the failure to the key actually dispatched, not pool.current(): + # the shared pointer often points at a different healthy entry, and marking + # it exhausted can take the whole pool offline from one 429 (#43747). _api_key_hint = getattr(agent, "api_key", None) or None _raw_credential_id = getattr(agent, "_credential_pool_entry_id", None) _credential_id = ( @@ -1189,21 +861,32 @@ def recover_with_credential_pool( } if _credential_id: kwargs["credential_id"] = _credential_id - # Hand the pool the classified semantics, not just the status. A - # billing 403 (OpenRouter "key limit exceeded", xAI spending limit) - # and an edge-throttle 403 are the same number but need opposite - # cooldowns — the pool can only tell them apart if we say which. - # ``effective_reason`` is resolved below; this closure runs after. + # Pass classified semantics, not just the status: a billing 403 and an + # edge-throttle 403 need opposite cooldowns. ``effective_reason`` is resolved below. if effective_reason is not None: _failure_reason = effective_reason.value if effective_reason == FailoverReason.billing and billing_unverified: - # Ambiguous billing body (#82154): persist the ambiguity so - # the cooldown is sized as transient, not a 1-hour bench. + # Ambiguous billing body (#82154): size the cooldown as transient, not a 1-hour bench. from agent.credential_pool import FAILURE_REASON_BILLING_UNVERIFIED _failure_reason = FAILURE_REASON_BILLING_UNVERIFIED kwargs["failure_reason"] = _failure_reason return pool.mark_exhausted_and_rotate(**kwargs) + def _rotate_and_swap(default_status: int, label: str) -> bool: + """Rotate away from the failed credential; True when a new entry was swapped in.""" + rotate_status = status_code if status_code is not None else default_status + next_entry = _rotate_failed_credential(rotate_status) + if next_entry is None: + return False + _ra().logger.info( + "Credential %s (%s) — rotated to pool entry %s", + rotate_status, + label, + getattr(next_entry, "id", "?"), + ) + agent._swap_credential(next_entry) + return True + effective_reason = classified_reason if effective_reason is None: if status_code == 402: @@ -1214,10 +897,8 @@ def recover_with_credential_pool( effective_reason = FailoverReason.auth if effective_reason == FailoverReason.upstream_rate_limit: - # An upstream provider (e.g. DeepSeek behind OpenRouter) is - # rate-limiting the aggregator's traffic — the user's credential is - # healthy. Do NOT rotate or mark exhausted; let the caller's fallback - # path switch to a different model entirely. + # Upstream (e.g. DeepSeek behind OpenRouter) is throttling the aggregator; the + # credential is healthy. Do not rotate/exhaust; let fallback switch models. upstream = (error_context or {}).get("upstream_provider") if error_context else None if upstream: _ra().logger.info( @@ -1233,28 +914,15 @@ def recover_with_credential_pool( return False, has_retried_429 if effective_reason == FailoverReason.billing: - rotate_status = status_code if status_code is not None else 402 - # Runtime credentials can be resolved by a separate pool instance, - # leaving this recovery pool without ``current_id``. Match the key - # that actually failed instead of quarantining a different account. - next_entry = _rotate_failed_credential(rotate_status) - if next_entry is not None: - _ra().logger.info( - "Credential %s (billing) — rotated to pool entry %s", - rotate_status, - getattr(next_entry, "id", "?"), - ) - agent._swap_credential(next_entry) + # A separate pool instance may have resolved runtime credentials, leaving + # no ``current_id``; match the key that failed, not a different account. + if _rotate_and_swap(402, "billing"): return True, False return False, has_retried_429 if effective_reason == FailoverReason.rate_limit: - # If current credential is already marked exhausted, skip retry and - # rotate immediately. This prevents the "cancel-between-429s" trap - # where has_retried_429 (a local var) gets reset on each new prompt, - # causing the pool to retry the same exhausted credential forever. - # Prefer the entry matching the failing key over the shared current() - # pointer, for the same attribution reason as above. + # Already-exhausted credential: rotate immediately. Avoids the "cancel-between-429s" + # trap where the local has_retried_429 resets per prompt and retries forever. current_entry = None if _credential_id: current_entry = next( @@ -1274,15 +942,7 @@ def recover_with_credential_pool( "Credential already exhausted (last_status=%s) — rotating immediately instead of retrying", current_last_status, ) - rotate_status = status_code if status_code is not None else 429 - next_entry = _rotate_failed_credential(rotate_status) - if next_entry is not None: - _ra().logger.info( - "Credential %s (rate limit, pre-exhausted) — rotated to pool entry %s", - rotate_status, - getattr(next_entry, "id", "?"), - ) - agent._swap_credential(next_entry) + if _rotate_and_swap(429, "rate limit, pre-exhausted"): return True, False return False, True @@ -1298,40 +958,15 @@ def recover_with_credential_pool( ) if not has_retried_429 and not usage_limit_reached: return False, True - rotate_status = status_code if status_code is not None else 429 - next_entry = _rotate_failed_credential(rotate_status) - if next_entry is not None: - _ra().logger.info( - "Credential %s (rate limit) — rotated to pool entry %s", - rotate_status, - getattr(next_entry, "id", "?"), - ) - agent._swap_credential(next_entry) + if _rotate_and_swap(429, "rate limit"): return True, False return False, True if effective_reason == FailoverReason.auth: - # Subscription/entitlement 403s look like auth failures on the wire - # but refresh cannot fix them — the OAuth token is already valid, - # the account simply lacks the entitlement. Without this guard, - # the refresh path keeps minting fresh tokens against the - # same unsubscribed account and the main agent loop spins re-issuing - # the same 403 until the user Ctrl+C's. - # - # Defense-in-depth for #26847: xAI's backend has been seen to 403 - # standard SuperGrok subscribers with bodies that don't match the - # existing entitlement keyword set in ``_is_entitlement_failure``. - # Any 403 against ``xai-oauth`` is treated as entitlement here so - # the refresh loop can't spin in those cases either. - # - # Exception (#29344): xAI's ``[WKE=unauthenticated:...]`` suffix and - # the ``OAuth2 access token could not be validated`` phrasing are - # xAI's authoritative "this is a stale token, not entitlement" - # signal. When either fires we must NOT apply the catch-all - # override — refresh is the recoverable path for these bodies, and - # blanket-classifying them as entitlement was the bug that left - # long-running TUI sessions stuck on stale tokens until the user - # exited and reopened. + # Entitlement 403s look like auth failures but refresh cannot fix them; any + # xai-oauth 403 is treated as entitlement (#26847) EXCEPT xAI's stale-token + # signals (``[WKE=unauthenticated:...]``, "could not be validated"), which must + # stay refreshable (#29344). is_entitlement = agent._is_entitlement_failure(error_context, status_code) _auth_haystack = " ".join( str(error_context.get(k) or "").lower() @@ -1367,22 +1002,15 @@ def recover_with_credential_pool( agent.provider or "provider", ) return False, has_retried_429 - # Refresh the entry that supplied the failing key, not current(): - # the shared pointer can reference a different, healthy entry, and - # refreshing it would consume that entry's single-use refresh token - # (or mark it exhausted on failure) for a failure it never had. + # Refresh the entry that supplied the failing key, not current(): refreshing a + # healthy entry burns its single-use refresh token for a failure it never had. refresh_kwargs = {"api_key_hint": _api_key_hint} if _credential_id: refresh_kwargs["credential_id"] = _credential_id refreshed = pool.try_refresh_matching(**refresh_kwargs) if refreshed is not None: - # ``try_refresh_matching()`` re-mints a fresh OAuth token and reports - # success even when the upstream keeps rejecting it — a single-entry - # pool (common for OAuth/Max subscribers) has nothing to rotate to, - # so a bare "refreshed → retry" loop spins forever on the same dead - # token and the configured fallback never activates. Cap consecutive - # same-entry refreshes and fall through to fallback once exceeded. - # See #26080. + # try_refresh_matching() reports success even when upstream keeps rejecting; + # cap same-entry refreshes so a single-entry pool falls through to fallback (#26080). refreshed_id = getattr(refreshed, "id", None) if refreshed_id is not None: refresh_counts = getattr(agent, "_auth_pool_refresh_counts", None) @@ -1403,42 +1031,55 @@ def recover_with_credential_pool( _ra().logger.info("Credential auth failure — refreshed pool entry %s", getattr(refreshed, 'id', '?')) agent._swap_credential(refreshed) return True, has_retried_429 - # Refresh failed — rotate to next credential instead of giving up. - # The failed entry is already marked exhausted by the refresh attempt. - rotate_status = status_code if status_code is not None else 401 - next_entry = _rotate_failed_credential(rotate_status) - if next_entry is not None: - _ra().logger.info( - "Credential %s (auth refresh failed) — rotated to pool entry %s", - rotate_status, - getattr(next_entry, "id", "?"), - ) - agent._swap_credential(next_entry) + # Refresh failed; rotate (the failed entry is already marked exhausted). + if _rotate_and_swap(401, "auth refresh failed"): return True, False return False, has_retried_429 +def _apply_primary_runtime_fields(agent, rt: Dict[str, Any]) -> None: + """Copy the identity/transport fields of a ``_primary_runtime`` snapshot onto ``agent``. + + Shared by transport recovery and turn-start restore; the caller rebuilds the client. + """ + agent.model = rt["model"] + agent.provider = rt["provider"] + agent.requested_provider = rt.get("requested_provider", agent.provider) + agent.base_url = rt["base_url"] # setter updates _base_url_lower + agent.api_mode = rt["api_mode"] + if hasattr(agent, "_transport_cache"): + agent._transport_cache.clear() + agent.api_key = rt["api_key"] + agent._reasoning_echo_flag = rt.get("reasoning_echo_flag", False) + agent.request_overrides = dict(rt.get("request_overrides") or {}) + agent._client_kwargs = dict(rt["client_kwargs"]) + + +def _build_anthropic_client_from_runtime(agent, rt: Dict[str, Any]) -> None: + """Rebuild the native Anthropic client from a ``_primary_runtime`` snapshot.""" + from agent.anthropic_adapter import build_anthropic_client + agent._anthropic_api_key = rt["anthropic_api_key"] + agent._anthropic_base_url = rt["anthropic_base_url"] + agent._anthropic_client = build_anthropic_client( + rt["anthropic_api_key"], rt["anthropic_base_url"], + timeout=get_provider_request_timeout(agent.provider, agent.model), + ) + agent._is_anthropic_oauth = rt["is_anthropic_oauth"] + agent.client = None + + def try_recover_primary_transport( agent, api_error: Exception, *, retry_count: int, max_retries: int, ) -> bool: - """Attempt one extra primary-provider recovery cycle for transient transport failures. + """Rebuild the primary client once and retry after ``max_retries`` exhaust on a transient transport error. - After ``max_retries`` exhaust, rebuild the primary client (clearing - stale connection pools) and give it one more attempt before falling - back. This is most useful for direct endpoints (custom, Z.AI, - Anthropic, OpenAI, local models) where a TCP-level hiccup does not - mean the provider is down. - - Skipped for proxy/aggregator providers (OpenRouter, Nous) which - already manage connection pools and retries server-side — if our - retries through them are exhausted, one more rebuilt client won't help. + Skipped for aggregator providers (OpenRouter, Nous) that already manage pools/retries server-side. """ if agent._fallback_activated: return False - # Only for transient transport errors error_type = type(api_error).__name__ if error_type not in _TRANSIENT_TRANSPORT_ERRORS: return False @@ -1447,11 +1088,8 @@ def try_recover_primary_transport( if agent._is_openrouter_url(): return False provider_lower = (agent.provider or "").strip().lower() - # Portal OpenAI-wire traffic still rides aggregator retry infra, so one - # more rebuilt OpenAI client won't help. Portal Claude on the native - # Messages route holds a local Anthropic SDK client whose connection - # pool *does* need the rebuild every other anthropic_messages provider - # already gets — don't blanket-skip the dual-wire path. + # Portal OpenAI-wire traffic rides aggregator retry infra (skip), but Portal + # Claude on native Messages holds a local Anthropic client that needs the rebuild. if ( provider_lower in {"nous", "nous-portal", "nousresearch"} and getattr(agent, "api_mode", None) != "anthropic_messages" @@ -1459,13 +1097,8 @@ def try_recover_primary_transport( return False try: - # Retire the existing client to release stale connections. #70773: - # never hard-close the shared client here — this runs on the - # conversation-loop thread while workers from stale-killed streaming - # attempts may still be unwinding their SSL BIOs on the old pool. - # ``_retire_shared_openai_client`` shuts the sockets down (FD-safe - # from any thread) and defers the FD release to GC, which cannot - # complete until every borrowing thread has unwound. + # Never hard-close the shared client here (#70773): stale streaming workers may + # still be unwinding on the old pool; _retire_shared_openai_client defers FD release to GC. if getattr(agent, "client", None) is not None: try: agent._retire_shared_openai_client( @@ -1474,35 +1107,14 @@ def try_recover_primary_transport( except Exception: pass - # Rebuild from primary snapshot rt = agent._primary_runtime - agent._client_kwargs = dict(rt["client_kwargs"]) - agent.model = rt["model"] - agent.provider = rt["provider"] - agent.requested_provider = rt.get("requested_provider", agent.provider) - agent.base_url = rt["base_url"] - agent.api_mode = rt["api_mode"] - if hasattr(agent, "_transport_cache"): - agent._transport_cache.clear() - agent.api_key = rt["api_key"] - agent._reasoning_echo_flag = rt.get("reasoning_echo_flag", False) - agent.request_overrides = dict(rt.get("request_overrides") or {}) + _apply_primary_runtime_fields(agent, rt) if agent.api_mode == "anthropic_messages": - from agent.anthropic_adapter import build_anthropic_client - agent._anthropic_api_key = rt["anthropic_api_key"] - agent._anthropic_base_url = rt["anthropic_base_url"] - agent._anthropic_client = build_anthropic_client( - rt["anthropic_api_key"], rt["anthropic_base_url"], - timeout=get_provider_request_timeout(agent.provider, agent.model), - ) - agent._is_anthropic_oauth = rt["is_anthropic_oauth"] - agent.client = None + _build_anthropic_client_from_runtime(agent, rt) elif (agent.provider or "").strip().lower() == "moa": - # MoA is a virtual provider with empty client_kwargs — rebuilding - # via _create_openai_client would raise "api_key client option - # must be set". Recreate the facade through the shared factory so - # the reference_callback relay survives recovery (#53802). + # MoA has empty client_kwargs; rebuild via the shared facade factory so the + # reference_callback relay survives recovery (#53802). from agent.moa_loop import build_moa_facade agent.client = build_moa_facade(agent, agent.model) @@ -1534,21 +1146,11 @@ def drop_thinking_only_and_merge_users( *, drop_codex_reasoning_items: bool = True, ) -> List[Dict[str, Any]]: - """Drop thinking-only assistant turns; merge any adjacent user messages left behind. + """Drop thinking-only assistant turns and merge adjacent user messages left behind. - Runs on the per-call ``api_messages`` copy only. The stored - conversation history (``agent.messages``) is never mutated, so the - user still sees the thinking block in the CLI/gateway transcript and - session persistence keeps the full trace. Only the wire copy sent to - the provider is cleaned. - - Why drop-and-merge rather than inject stub text: - - Fabricating ``"."`` / ``"(continued)"`` text lies in the history - and makes future turns see model output the model didn't emit. - - Dropping the turn preserves honesty; merging adjacent user messages - preserves the provider's role-alternation invariant. - - This is the pattern used by Claude Code's ``normalizeMessagesForAPI`` - (filterOrphanedThinkingOnlyMessages + mergeAdjacentUserMessages). + Operates on the per-call ``api_messages`` copy only; ``agent.messages`` is never mutated. + Drop-and-merge (not stub text) keeps history honest and preserves role alternation + (mirrors Claude Code's ``normalizeMessagesForAPI``). """ if not messages: return messages @@ -1575,14 +1177,9 @@ def drop_thinking_only_and_merge_users( ): prev_content = prev.get("content", "") cur_content = m.get("content", "") - # Work on a copy of ``prev`` so the caller's input dicts are - # never mutated. ``_sanitize_api_messages`` upstream already - # hands us per-call copies, but staying pure here means we - # can be called safely from anywhere (tests, other loops). + # Copy ``prev`` so caller dicts are never mutated (safe from tests/other loops). prev_copy = dict(prev) - # Only string-content merge is meaningful for role-alternation - # purposes. If either side is a list (multimodal), append as a - # separate block rather than collapsing. + # Only string+string content merges; list (multimodal) sides append as separate blocks. if isinstance(prev_content, str) and isinstance(cur_content, str): sep = "\n\n" if prev_content and cur_content else "" prev_copy["content"] = prev_content + sep + cur_content @@ -1625,85 +1222,47 @@ def drop_thinking_only_and_merge_users( def restore_primary_runtime(agent) -> bool: - """Restore the primary runtime at the start of a new turn. + """Restore the primary runtime at the start of a new turn so fallback stays turn-scoped. - In long-lived CLI sessions a single AIAgent instance spans multiple - turns. Without restoration, one transient failure pins the session - to the fallback provider for every subsequent turn. Calling this at - the top of ``run_conversation()`` makes fallback turn-scoped. - - The gateway caches agents across messages (``_agent_cache`` in - ``gateway/run.py``), so this restoration IS needed there too. + Needed for long-lived CLI agents and the gateway's cached agents (``_agent_cache``). """ if not agent._fallback_activated: - # Reset the chain index even when no fallback was activated this - # turn. Without this, a turn where _try_activate_fallback() was - # called but returned False (chain exhausted or provider not - # configured) leaves _fallback_index >= len(_fallback_chain) while - # _fallback_activated stays False. The next turn skips this block - # entirely, stranding the index and silently blocking all future - # fallback attempts for the session. Fixes #20465. + # Reset the index even without activation: a failed _try_activate_fallback() can strand + # _fallback_index past the chain end and silently block future fallbacks (#20465). agent._fallback_index = 0 return False if getattr(agent, "_rate_limited_until", 0) > time.monotonic(): return False # primary still in rate-limit cooldown, stay on fallback - # ── Reset-aware gate ── - # The 60s ``_rate_limited_until`` cooldown covers transient rate limits, - # but subscription-style providers (Claude Pro/Max 5-hour windows, ChatGPT - # weekly limits) report reset times hours or days away. The credential - # pool already stores those timestamps (``last_error_reset_at``); until - # the earliest one elapses, every restore attempt is a *guaranteed* - # failure that costs two prompt-cache invalidations per turn (switch to - # primary, fail, switch back to fallback) and re-marshals the full - # context each way. Skip the restore while the pool says nobody can - # serve, and come back the moment the reset time passes. - # - # Fail-open by design: any error (unreadable auth store, legacy pool - # adapter without ``next_available_at``) falls through to the existing - # every-turn retry. A pool with no reset info returns ``None`` and also - # falls through — this gate only ever *adds* skips for provably - # limited windows, so recovery can never be later than it is today. - # - # When the attached pool belongs to the fallback provider (cross-provider - # fallback rebinds it), the primary pool is loaded here and handed to the - # pool-rebind block below via ``prefetched_primary_pool`` so the load - # happens at most once per restore. + # Reset-aware gate: when the credential pool reports a reset time still in the future + # (subscription windows), skip the guaranteed-to-fail restore (saves two cache invalidations + # per turn). Fails open on any error/None. The loaded primary pool is handed to the + # rebind block below via ``prefetched_primary_pool`` so it loads at most once. + rt = agent._primary_runtime + primary_provider = str((rt or {}).get("provider") or "").strip().lower() + primary_runtime_base_url = str((rt or {}).get("base_url") or "") + + def _matches_primary(candidate) -> bool: + return credential_pool_matches_provider( + candidate, primary_provider, base_url=primary_runtime_base_url + ) + + def _load_primary_pool(): + """Load the primary provider's pool; None when absent or provider-mismatched.""" + from agent.credential_pool import load_pool + + key = resolve_runtime_pool_key(primary_provider, primary_runtime_base_url) + loaded = load_pool(key) if key else None + return loaded if loaded is not None and _matches_primary(loaded) else None + prefetched_primary_pool = None primary_pool_prefetched = False try: - primary_provider = str( - (agent._primary_runtime or {}).get("provider") or "" - ).strip().lower() - primary_runtime_base_url = str( - (agent._primary_runtime or {}).get("base_url") or "" - ) - primary_pool_key = resolve_runtime_pool_key( - primary_provider, - primary_runtime_base_url, - ) pool = getattr(agent, "_credential_pool", None) - if not credential_pool_matches_provider( - pool, - primary_provider, - base_url=primary_runtime_base_url, - ): - from agent.credential_pool import load_pool - - prefetched_primary_pool = ( - load_pool(primary_pool_key) if primary_pool_key else None - ) + if not _matches_primary(pool): + prefetched_primary_pool = pool = _load_primary_pool() primary_pool_prefetched = True - if prefetched_primary_pool is not None and credential_pool_matches_provider( - prefetched_primary_pool, - primary_provider, - base_url=primary_runtime_base_url, - ): - pool = prefetched_primary_pool - else: - prefetched_primary_pool = None - pool = None next_at = getattr(pool, "next_available_at", lambda: None)() if next_at is not None and next_at > time.time(): if not getattr(agent, "_restore_wait_logged", False): @@ -1724,7 +1283,6 @@ def restore_primary_runtime(agent) -> bool: ) agent._restore_wait_logged = False - rt = agent._primary_runtime fallback_route = getattr(agent, "_provider_fallback_route", None) if ( isinstance(fallback_route, (list, tuple)) @@ -1740,14 +1298,7 @@ def restore_primary_runtime(agent) -> bool: ) try: # ── Core runtime state ── - agent.model = rt["model"] - agent.provider = rt["provider"] - agent.requested_provider = rt.get("requested_provider", agent.provider) - agent.base_url = rt["base_url"] # setter updates _base_url_lower - agent.api_mode = rt["api_mode"] - if hasattr(agent, "_transport_cache"): - agent._transport_cache.clear() - agent.api_key = rt["api_key"] + _apply_primary_runtime_fields(agent, rt) if "runtime_capabilities" in rt: raw_capabilities = rt["runtime_capabilities"] if not isinstance(raw_capabilities, dict): @@ -1759,45 +1310,27 @@ def restore_primary_runtime(agent) -> bool: raw_capabilities = rt["capabilities"] if isinstance(raw_capabilities, dict): agent.runtime_capabilities = dict(raw_capabilities) - agent._reasoning_echo_flag = rt.get("reasoning_echo_flag", False) - agent.request_overrides = dict(rt.get("request_overrides") or {}) - agent._client_kwargs = dict(rt["client_kwargs"]) agent._use_prompt_caching = rt["use_prompt_caching"] - # Default to native layout when the restored snapshot predates the - # native-vs-proxy split (older sessions saved before this PR). + # Default to native layout for snapshots predating the native-vs-proxy split. agent._use_native_cache_layout = rt.get( "use_native_cache_layout", agent.api_mode == "anthropic_messages" and agent.provider == "anthropic", ) - # If the operator has disabled caching via config (cache_ttl is - # falsy → _cache_disabled flag is set), the disable must survive - # runtime snapshot restoration (#33555). + # An operator cache disable (_cache_disabled) must survive snapshot restoration (#33555). if getattr(agent, "_cache_disabled", False): agent._use_prompt_caching = False agent._use_native_cache_layout = False # ── Rebuild client for the primary provider ── if agent.provider == "moa": - # MoA is a virtual chat-completions provider. It never has real - # OpenAI client kwargs; restoring it after a fallback must recreate - # the facade, not call OpenAI() with an empty api_key. Use the - # shared factory so the restored facade keeps the reference_callback - # relay wired at init — a bare MoAClient() would silently stop - # emitting moa.reference/moa.aggregating display events (#53802). + # MoA has no real OpenAI client kwargs; rebuild via the shared facade factory so the + # reference_callback relay stays wired (#53802). from agent.moa_loop import build_moa_facade agent.client = build_moa_facade(agent, agent.model) agent._anthropic_client = None elif agent.api_mode == "anthropic_messages": - from agent.anthropic_adapter import build_anthropic_client - agent._anthropic_api_key = rt["anthropic_api_key"] - agent._anthropic_base_url = rt["anthropic_base_url"] - agent._anthropic_client = build_anthropic_client( - rt["anthropic_api_key"], rt["anthropic_base_url"], - timeout=get_provider_request_timeout(agent.provider, agent.model), - ) - agent._is_anthropic_oauth = rt["is_anthropic_oauth"] - agent.client = None + _build_anthropic_client_from_runtime(agent, rt) else: agent.client = agent._create_openai_client( dict(rt["client_kwargs"]), @@ -1817,50 +1350,18 @@ def restore_primary_runtime(agent) -> bool: ) # ── Rebind and re-select the primary credential pool ── - # A cross-provider fallback attaches the fallback provider's pool. The - # runtime fields above restore the primary, but leaving that pool in - # place makes the next primary 401/429 hit the provider-mismatch guard - # and disables credential rotation. Reload the primary pool first; if - # auth storage is temporarily unreadable, clear the mismatched pool. - primary_provider = str(rt.get("provider") or "").strip().lower() - primary_runtime_base_url = str(rt.get("base_url") or "") - primary_pool_key = resolve_runtime_pool_key( - primary_provider, - primary_runtime_base_url, - ) + # A cross-provider fallback attaches the fallback's pool; leaving it would trip the + # provider-mismatch guard on the next 401/429. Reload the primary pool, else clear it. pool = getattr(agent, "_credential_pool", None) pool_provider = str(getattr(pool, "provider", "") or "").strip().lower() - pool_matches_primary = credential_pool_matches_provider( - pool, - primary_provider, - base_url=primary_runtime_base_url, - ) - if pool is not None and pool_provider and not pool_matches_primary: + if pool is not None and pool_provider and not _matches_primary(pool): agent._credential_pool = None agent._credential_pool_entry_id = None try: - if primary_pool_prefetched: - # Reuse the pool the reset-aware gate already loaded for - # this restore — avoids a second disk read of auth.json. - if ( - prefetched_primary_pool is not None - and credential_pool_matches_provider( - prefetched_primary_pool, - primary_provider, - base_url=primary_runtime_base_url, - ) - ): - agent._credential_pool = prefetched_primary_pool - else: - from agent.credential_pool import load_pool - - loaded_pool = load_pool(primary_pool_key) - if loaded_pool is not None and credential_pool_matches_provider( - loaded_pool, - primary_provider, - base_url=primary_runtime_base_url, - ): - agent._credential_pool = loaded_pool + # Reuse the pool the reset-aware gate already loaded (avoids a second auth.json read). + agent._credential_pool = ( + prefetched_primary_pool if primary_pool_prefetched else _load_primary_pool() + ) except Exception as exc: logger.warning( "Restore could not reload primary credential pool for %s: %s", @@ -1868,34 +1369,20 @@ def restore_primary_runtime(agent) -> bool: exc, ) - # The snapshot's api_key was captured at construction time. Across - # turns the pool may have rotated (token revocation, billing/rate-limit - # exhaustion, cooldown), leaving the snapshot key stale. Restoring it - # blindly re-fails on the first request and burns through the remaining - # pool entries before cross-provider fallback even gets a chance. Ask - # the pool for its current best entry and swap the live credential in. - # When the pool is absent, empty, or the entry has no usable key, we - # keep the snapshot key (the existing behavior). Fixes #25205. + # The snapshot api_key may be stale after pool rotation; re-select the pool's current + # best entry, keeping the snapshot key when no usable entry exists (#25205). agent._credential_pool_entry_id = None pool = getattr(agent, "_credential_pool", None) if pool is not None and pool.has_available(): entry = pool.select() if entry is not None: entry_provider = str(getattr(entry, "provider", "") or "").strip().lower() - entry_matches_primary = credential_pool_matches_provider( - entry, - primary_provider, - base_url=primary_runtime_base_url, - ) - entry_key = ( getattr(entry, "runtime_api_key", None) or getattr(entry, "access_token", "") ) - if entry_key and entry_matches_primary: - # ``_swap_credential`` rebuilds the OpenAI/Anthropic client, - # reapplies base-url-scoped headers, and carries the - # accumulated base_url / OAuth-detection fixes (#33163). + if entry_key and _matches_primary(entry): + # _swap_credential rebuilds the client and reapplies base-url-scoped headers (#33163). agent._swap_credential(entry) logger.info( "Restore re-selected pool entry %s (%s)", @@ -1911,9 +1398,7 @@ def restore_primary_runtime(agent) -> bool: primary_provider or "?", ) - # ── Restore reasoning_config if it was saved ── - # switch_model saves reasoning_config in _primary_runtime. If the - # snapshot predates that (older sessions), keep the current value. + # ── Restore reasoning_config if saved (older snapshots keep the current value) ── saved_reasoning = rt.get("reasoning_config") if saved_reasoning is not None: agent.reasoning_config = dict(saved_reasoning) @@ -1923,9 +1408,7 @@ def restore_primary_runtime(agent) -> bool: agent._fallback_index = 0 agent._rate_limit_backoff_count = 0 # reset exponential backoff counter - # Reset the stale-call circuit breaker (#58962): the streak measured - # the FALLBACK provider we're leaving; the restored primary deserves - # a fresh stream attempt before the breaker can trip again. + # Reset the stale-call circuit breaker (#58962): its streak measured the fallback provider. from agent.chat_completion_helpers import _reset_stale_streak _reset_stale_streak(agent) @@ -1966,38 +1449,24 @@ _TRANSIENT_TRANSPORT_ERRORS = frozenset({ def extract_reasoning(agent, assistant_message) -> Optional[str]: - """ - Extract reasoning/thinking content from an assistant message. - - OpenRouter and various providers can return reasoning in multiple formats: - 1. message.reasoning - Direct reasoning field (DeepSeek, Qwen, etc.) - 2. message.reasoning_content - Alternative field (Moonshot AI, Novita, etc.) - 3. message.reasoning_details - Array of {type, summary, ...} objects (OpenRouter unified) - - Args: - assistant_message: The assistant message object from the API response - - Returns: - Combined reasoning text, or None if no reasoning found + """Extract reasoning text from an assistant message, or None. + + Checks ``reasoning``, ``reasoning_content``, ``reasoning_details`` (OpenRouter unified), + then inline thinking blocks in list content. """ reasoning_parts = [] - # Check direct reasoning field if hasattr(assistant_message, 'reasoning') and assistant_message.reasoning: reasoning_parts.append(assistant_message.reasoning) - # Check reasoning_content field (alternative name used by some providers) if hasattr(assistant_message, 'reasoning_content') and assistant_message.reasoning_content: - # Don't duplicate if same as reasoning if assistant_message.reasoning_content not in reasoning_parts: reasoning_parts.append(assistant_message.reasoning_content) - # Check reasoning_details array (OpenRouter unified format) - # Format: [{"type": "reasoning.summary", "summary": "...", ...}, ...] + # reasoning_details: [{"type": "reasoning.summary", "summary": "...", ...}, ...] if hasattr(assistant_message, 'reasoning_details') and assistant_message.reasoning_details: for detail in assistant_message.reasoning_details: if isinstance(detail, dict): - # Extract summary from reasoning detail object summary = ( detail.get('summary') or detail.get('thinking') @@ -2007,17 +1476,11 @@ def extract_reasoning(agent, assistant_message) -> Optional[str]: if summary and summary not in reasoning_parts: reasoning_parts.append(summary) - # Some providers embed reasoning directly inside assistant content - # instead of returning structured reasoning fields. Only fall back - # to inline extraction when no structured reasoning was found. + # Fall back to reasoning embedded in content only when no structured field 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. + # DeepSeek V4 Pro returns typed content blocks ({"type": "thinking", ...}); dropping + # them makes the next turn fail with HTTP 400 "thinking must be passed back" (#21944). for block in content: if isinstance(block, dict) and block.get("type") == "thinking": thinking_text = block.get("thinking") or block.get("text") or "" @@ -2039,7 +1502,6 @@ def extract_reasoning(agent, assistant_message) -> Optional[str]: if cleaned and cleaned not in reasoning_parts: reasoning_parts.append(cleaned) - # Combine all reasoning parts if reasoning_parts: return "\n\n".join(reasoning_parts) @@ -2054,13 +1516,7 @@ def dump_api_request_debug( reason: str, error: Optional[Exception] = None, ) -> Optional[Path]: - """ - Dump a debug-friendly HTTP request record for the active inference API. - - Captures the request body from api_kwargs (excluding transport-only keys - like timeout). Intended for debugging provider-side 4xx failures where - retries are not useful. - """ + """Dump the request body from api_kwargs (minus transport keys) for debugging provider 4xx failures.""" try: body = copy.deepcopy(api_kwargs) body.pop("timeout", None) @@ -2112,19 +1568,13 @@ def dump_api_request_debug( dump_payload["error"] = error_info timestamp = datetime.now().strftime("%Y%m%d_%H%M%S_%f") - # Sanitize the session ID into a traversal-free path segment — it can - # originate from untrusted input (X-Hermes-Session-Id header), and an - # unsanitized "../"-shaped ID would write the dump outside logs_dir. + # Sanitize the session ID (may come from an untrusted X-Hermes-Session-Id header) + # so a "../"-shaped ID cannot write outside logs_dir. safe_sid = _ra()._safe_session_filename_component(agent.session_id) dump_file = agent.logs_dir / f"request_dump_{safe_sid}_{timestamp}.json" - # Redact secrets before persisting/printing. This dump captures the - # full request body (system prompt, tool defs, context-embedded - # values), and this path fires unconditionally on API errors — so it - # otherwise lands any context-embedded secret in cleartext on disk. - # Run the serialized dump through the same scrubber used for logs/tool - # output, then hand the resulting payload back to the shared atomic - # JSON writer so request dumps keep the same write semantics as before. + # Redact secrets first: this fires unconditionally on API errors and captures the full + # request body, so context-embedded secrets would otherwise land in cleartext on disk. from agent.redact import redact_sensitive_text _serialized = json.dumps(dump_payload, ensure_ascii=False, indent=2, default=str) _redacted_payload = json.loads(redact_sensitive_text(_serialized, force=True)) @@ -2163,13 +1613,8 @@ def _direct_native_anthropic_tool_cache_capability( def cache_ttl_means_disabled(ttl: Any) -> bool: """Return True when a ``prompt_caching.cache_ttl`` value means caching off. - Single source of truth for the disable-synonym detection shared by - ``agent_init`` (live-agent ``_cache_disabled`` flag) and the stub policy - paths below. Keeping one predicate prevents the two sites from drifting - (a synonym added in only one place would recreate #76085). - - Unknown values (e.g. ``"2h"``, integers) are NOT a disable — callers keep - caching enabled with the default TTL, matching ``agent_init``. + Single predicate shared by ``agent_init`` and the stub policy paths (#76085). + Unknown values (``"2h"``, integers) are NOT a disable. """ if ttl in ("5m", "1h"): return False @@ -2178,9 +1623,7 @@ def cache_ttl_means_disabled(ttl: Any) -> bool: return str(ttl).lower() in ("off", "false", "disabled", "no", "none") -# The two cache_ttl tiers accepted by config (anything else is either a -# disable synonym or ignored). Shared by the config readers below and -# mirrored by agent_init's live-agent snapshot. +# The cache_ttl tiers accepted by config; mirrored by agent_init's live-agent snapshot. VALID_CACHE_TTLS = ("5m", "1h") @@ -2193,13 +1636,7 @@ def _raw_cache_ttl_from_config() -> Any: def prompt_caching_disabled_from_config() -> bool: - """Return True when ``prompt_caching.cache_ttl`` is configured as off. - - Same disable detection as ``agent_init`` (via ``cache_ttl_means_disabled``) - so stub-based policy paths (MoA slot decoration, auxiliary fallback - replan) honor the same config contract without holding a live - ``AIAgent`` (#76085 / #33555). - """ + """Return True when ``prompt_caching.cache_ttl`` is configured as off (same detection as ``agent_init``; #76085 / #33555).""" try: ttl = _raw_cache_ttl_from_config() except Exception: @@ -2208,13 +1645,9 @@ def prompt_caching_disabled_from_config() -> bool: def configured_cache_ttl() -> Optional[str]: - """Return the configured ``prompt_caching.cache_ttl`` tier, if valid. + """Return the configured ``prompt_caching.cache_ttl`` tier (``5m``/``1h``), else None. - Mirrors ``agent_init``'s reading of the same key (``5m``/``1h`` accepted, - anything else ignored) so stub-based paths without a live ``AIAgent`` - (auxiliary fallback replan) stop regressing a configured ``1h`` to the - 5m default (#84733). Returns ``None`` for unset/disabled/unknown values; - ``effective_cache_ttl`` resolves ``None`` to ``5m`` downstream. + Mirrors ``agent_init`` so stub paths don't regress a configured ``1h`` to 5m (#84733). """ try: ttl = _raw_cache_ttl_from_config() @@ -2226,13 +1659,8 @@ def configured_cache_ttl() -> Optional[str]: def blank_cache_policy_stub(cache_disabled: Optional[bool] = None): """Build the destination-identity-blank stub for ``anthropic_prompt_cache_policy``. - Single sanctioned constructor for that stub. Callers that resolve cache - policy against a destination identified out-of-band (not a live - ``AIAgent``) must go through here so ``_cache_disabled`` is never left - off a hand-rolled ``SimpleNamespace`` (#76085). - - When ``cache_disabled`` is omitted, falls back to the global config so - stub paths without an agent snapshot still honor an operator disable. + Sole sanctioned constructor so ``_cache_disabled`` is never omitted (#76085); when + ``cache_disabled`` is None the global config is consulted. """ from types import SimpleNamespace @@ -2259,30 +1687,12 @@ def plan_cache_sections_for_destination( cache_ttl: Optional[str] = None, static_system_prefix: Optional[str] = None, ) -> Tuple[list, list]: - """Plan request-local cache sections for one resolved destination. + """Plan request-local cache sections for one resolved destination (MoA / auxiliary senders). - Shared core of the synchronous acting-aggregator (MoA) and auxiliary - fallback senders: resolve the cache policy for the destination's real - provider/base_url/api_mode/model, then either return stripped canonical - copies (non-caching route) or a :func:`build_prompt_cache_plan` layout - (caching route, with the direct-native tool marker when the destination - is api.anthropic.com on the Messages wire). - - Never mutates ``messages`` or ``tools`` — both return values are - request-local copies. - - ``cache_disabled`` threads the operator's ``prompt_caching.cache_ttl`` - disable into the blank policy stub. When omitted, the live config is - consulted so MoA/auxiliary paths cannot re-enable markers after the - user turned caching off (#76085). - - ``cache_ttl`` threads the operator's configured tier (default ``5m``) - into the destination plan so MoA/auxiliary requests stop regressing to - the 5m default while the main loop honors ``1h`` (#84733); it is - clamped per-destination by :func:`effective_cache_ttl` (Qwen → 5m). - ``static_system_prefix`` threads the builder-declared stable prefix so - the destination system prompt receives the same early breakpoint the - main loop applies instead of marking the whole prompt as a breakpoint. + Returns stripped copies (non-caching route) or a ``build_prompt_cache_plan`` layout; never + mutates ``messages``/``tools``. ``cache_disabled`` and ``cache_ttl`` default to live config + so these paths honor the operator's disable (#76085) and tier (#84733); + ``static_system_prefix`` gives the system prompt the same early breakpoint as the main loop. """ from agent.prompt_caching import ( build_prompt_cache_plan, @@ -2308,10 +1718,7 @@ def plan_cache_sections_for_destination( messages, tools, cache_ttl=effective_cache_ttl( - # effective_cache_ttl resolves None → "5m"; markers are only - # emitted at all when should_cache passed above, so a - # cache-disabled agent (_cache_ttl=None) never reaches here - # with caching active. + # effective_cache_ttl resolves None → "5m"; cache-disabled agents never reach here. cache_ttl, provider=provider, model=model, @@ -2339,14 +1746,8 @@ def plan_cache_sections_for_destination( def _is_litellm_route(provider_lower: str, base_url: str) -> bool: """True when a route is a LiteLLM proxy, by provider id or host token. - Provider naming varies per install (``litellm``, ``custom:litellm``, or a - bare ``custom`` alias pointed at a LiteLLM host), so both signals are - checked. Both match ``litellm`` as a whole delimited token rather than a - raw substring: ``base_url_hostname``'s own docstring names substring host - matching as the false-positive class to avoid, and a plain - ``"litellm" in ...`` grants Anthropic markers to unrelated routes like - ``notlitellm.example.com`` or a provider named ``custom:notlitellm``. - A ``litellm`` *path* segment never qualifies — only the host does. + ``litellm`` must match as a whole delimited token (not substring) in provider or host; + a path segment never qualifies. """ if _has_litellm_token(provider_lower, ":-_/"): return True @@ -2370,39 +1771,13 @@ def anthropic_prompt_cache_policy( api_mode: Optional[str] = None, model: Optional[str] = None, ) -> tuple[bool, bool]: - """Decide whether to apply Anthropic prompt caching and which layout to use. + """Decide whether to apply Anthropic prompt caching; returns ``(should_cache, use_native_layout)``. - Returns ``(should_cache, use_native_layout)``: - * ``should_cache`` — inject ``cache_control`` breakpoints for this - request (applies to OpenRouter Claude, native Anthropic, and - third-party gateways that speak the native Anthropic protocol). - * ``use_native_layout`` — place markers on the *inner* content - blocks (native Anthropic accepts and requires this layout); - when False markers go on the message envelope (OpenRouter and - OpenAI-wire proxies expect the looser layout). - - Third-party providers using the native Anthropic transport - (``api_mode == 'anthropic_messages'`` + Claude-named model) get - caching with the native layout so they benefit from the same - cost reduction as direct Anthropic callers, provided their - gateway implements the Anthropic cache_control contract - (MiniMax, Zhipu GLM, LiteLLM's Anthropic proxy mode all do). - - Qwen / Alibaba-family models on OpenCode, OpenCode Go, and direct - Alibaba (DashScope) also honour Anthropic-style ``cache_control`` - markers on OpenAI-wire chat completions. Upstream pi-mono #3392 / - pi #3393 documented this for opencode-go Qwen. Without markers - these providers serve zero cache hits, re-billing the full prompt - on every turn. - - If the operator has set ``prompt_caching.cache_ttl`` to a falsy value - (``false``, ``null``, ``"off"``, etc.) in config.yaml, prompt caching - is fully disabled — this early return ensures the disable survives - ``/model`` switches, fallback re-derivation, and runtime snapshot - restoration (#33555). We check ``"_cache_disabled"`` (set by - init_agent when the disable is detected) rather than ``_cache_ttl`` - directly, because ``_cache_ttl`` is not yet set when the policy runs - during the initial ``init_agent`` call. + ``use_native_layout`` puts markers on inner content blocks (native Anthropic wire); + otherwise on the message envelope (OpenRouter / OpenAI-wire proxies). Qwen/Alibaba routes + also honour envelope markers (pi-mono #3392). An operator disable is read from + ``_cache_disabled`` (not ``_cache_ttl``, unset during init) so it survives switches and + restores (#33555). """ if getattr(agent, "_cache_disabled", False): return (False, False) @@ -2412,12 +1787,8 @@ def anthropic_prompt_cache_policy( eff_api_mode = api_mode if api_mode is not None else (agent.api_mode or "") eff_model = (model if model is not None else agent.model) or "" - # MoA virtual provider: the agent's model/provider are the preset name and - # "moa" — neither matches any caching branch, so the ACTING AGGREGATOR - # (often Claude on OpenRouter) silently lost prompt caching entirely - # (measured: 85% cache share solo vs 2% on the identical model via MoA — - # tens of millions of re-billed input tokens per benchmark run). Resolve - # the policy from the preset's real aggregator slot instead. + # MoA virtual provider matches no caching branch, silently losing caching for the acting + # aggregator; resolve the policy from the preset's real aggregator slot instead. if eff_provider.strip().lower() == "moa": try: from hermes_cli.config import load_config as _load_moa_cfg @@ -2458,21 +1829,14 @@ def anthropic_prompt_cache_policy( model_lower = eff_model.lower() provider_lower = eff_provider.lower() is_claude = "claude" in model_lower - # Kimi / Moonshot family via OpenRouter: same cache_control wire format - # as Claude on OpenRouter (envelope layout). Without this branch - # moonshotai/kimi-k2.6 falls through to (False, False), serving ~1% - # cache hits on 64K-token prompts and re-billing the full prompt on - # every turn. Observed within-turn progression with cache enabled: - # 1% → 67% → 84% → 97% (#25970). Reuses the canonical family matcher - # (covers bare k1./k2./k25 release slugs the substring check missed). + # Kimi/Moonshot via OpenRouter uses the same envelope cache_control as Claude; without + # this branch it serves ~1% cache hits (#25970). Family matcher covers bare k1./k2. slugs. from agent.anthropic_adapter import _model_name_is_kimi_family is_kimi = ( _model_name_is_kimi_family(eff_model) or "moonshot" in model_lower ) is_openrouter = base_url_host_matches(eff_base_url, "openrouter.ai") - # Nous Portal proxies to OpenRouter behind the scenes — identical - # OpenAI-wire envelope cache_control semantics. Treat it as an - # OpenRouter-equivalent endpoint for caching layout purposes. + # Nous Portal proxies to OpenRouter; treat as OpenRouter-equivalent for cache layout. is_nous_portal = base_url_host_matches(eff_base_url, "nousresearch.com") is_anthropic_wire = eff_api_mode == "anthropic_messages" is_native_anthropic = ( @@ -2480,14 +1844,8 @@ def anthropic_prompt_cache_policy( and (eff_provider == "anthropic" or base_url_hostname(eff_base_url) == "api.anthropic.com") ) - # A configured route may use an arbitrary provider name and model alias - # that are canonicalized only after Hermes sends the request. Honor its - # existing per-model ``prompt_caching`` capability instead of guessing - # support from either spelling. Explicit false is authoritative too. - # - # The declaration only controls the two transports handled by this marker - # planner. Responses and Bedrock use separate caching protocols and must - # not receive Anthropic-style cache_control fields. + # Honor a configured route's per-model ``prompt_caching`` capability (explicit false too); + # only for the two transports this planner handles, not Responses/Bedrock. custom_prompt_caching = None _supports_anthropic_cache_markers = eff_api_mode in { "anthropic_messages", @@ -2505,17 +1863,8 @@ def anthropic_prompt_cache_policy( # identity probe entirely for those transports. pass elif _custom_providers: - # The normalized list is already attached after agent initialization. - # Use cheap runtime identity signals before calling the capability - # helper so an unrelated configured provider does not put every - # built-in chat-completions request on the route-normalization path. - # - # Identity must match the authoritative helper's semantics: - # get_custom_provider_model_capability compares base URLs via - # normalize_route_base_url, and runtime provider ids go through - # custom_provider_aliases (space→hyphen, custom: prefix variants). - # A raw-string gate here would silently drop declarations whose - # config spelling differs only in host case / trailing slash. + # Cheap identity gate before the capability helper, matching its semantics + # (normalize_route_base_url + custom_provider_aliases) so spelling differences don't drop declarations. from hermes_cli.providers import custom_provider_aliases from hermes_cli.route_identity import normalize_route_base_url @@ -2538,19 +1887,13 @@ def anthropic_prompt_cache_policy( _route_may_be_custom = True break elif _custom_providers is None: - # None = the list is not attached yet (early agent initialization or - # a blank_cache_policy_stub destination); an attached empty list means - # the agent initialized with no custom providers and correctly never - # matches. Avoid rebuilding the list for ordinary built-in routes, - # while still recognizing arbitrary config keys and built-in-name - # overrides that point at a different endpoint. + # None = list not attached yet (early init or blank stub); an attached empty list never + # matches. Avoid rebuilding the list for ordinary built-in routes. try: from hermes_cli.providers import get_provider - # allow_network=False: this runs per request destination; a cold - # models.dev cache must not trigger a foreground registry fetch - # from the send path. A catalog miss (None) degrades to the - # conservative side (route may be custom → capability lookup). + # allow_network=False: never trigger a registry fetch from the send path; + # a catalog miss degrades to the conservative capability lookup. _provider_def = get_provider(eff_provider, allow_network=False) _route_may_be_custom = _provider_def is None or ( bool(_provider_def.base_url) @@ -2582,20 +1925,11 @@ def anthropic_prompt_cache_policy( _cap_exc, ) if custom_prompt_caching is not None: - # Layout follows the transport, not the declaration: native Messages - # uses inner-block markers; OpenAI-compatible chat uses the envelope - # layout already emitted for OpenRouter and LiteLLM. + # Layout follows the transport: native Messages → inner blocks; OpenAI wire → envelope. return custom_prompt_caching, custom_prompt_caching and is_anthropic_wire - # MiniMax-M3 rides MiniMax's server-side automatic prefix cache on the - # Anthropic wire (content-keyed, no marker needed); explicit cache_control - # is documented for M2.7/M2.5/M2.1/M2 only, so markers on M3 are dead - # weight — never observable (cache_creation always 0) nor billable. - # Checked BEFORE the native-Anthropic return: provider="anthropic" - # pointed at a MiniMax /anthropic proxy is a supported override - # (_anthropic_base_url_override_ok) that would otherwise return - # (True, True) above this exclusion. - # Docs: https://platform.minimax.io/docs/api-reference/text-prompt-caching + # MiniMax-M3 uses server-side automatic prefix caching; explicit markers are dead weight. + # Checked BEFORE the native-Anthropic return since provider="anthropic" may point at a MiniMax proxy. is_minimax_provider = provider_lower in {"minimax", "minimax-cn"} is_minimax_host = ( base_url_host_matches(eff_base_url, "api.minimax.io") @@ -2610,99 +1944,44 @@ def anthropic_prompt_cache_policy( if is_native_anthropic: return True, True - # Envelope layout is an OpenAI-wire construct. Portal Claude on the native - # Messages route must fall through to the third-party anthropic_messages - # branch below, which emits inner-block cache_control breakpoints; the - # envelope form would be dropped and serve 0% cache hits. + # Envelope layout is OpenAI-wire only; Portal Claude on native Messages must fall through + # to the anthropic_messages branch (inner-block markers) or it serves 0% cache hits. if ( (is_openrouter or is_nous_portal) and (is_claude or is_kimi) and not is_anthropic_wire ): return True, False - # Nous Portal Qwen (e.g. qwen3.6-plus) takes the same envelope-layout - # cache_control path as Portal Claude. Portal proxies to OpenRouter - # and the upstream Qwen route accepts cache_control markers; without - # this branch the alibaba-family check below only matches - # provider=opencode/alibaba and Portal traffic falls through to - # (False, False), serving 0% cache hits and re-billing the full - # prompt on every turn. + # Nous Portal Qwen takes the envelope path too; the alibaba-family check below only matches + # provider=opencode/alibaba and would leave Portal traffic uncached. if is_nous_portal and "qwen" in model_lower: return True, False if is_anthropic_wire and is_claude: # Third-party Anthropic-compatible gateway. return True, True - # LiteLLM fronting a Claude model on the OpenAI-compatible wire. - # The branch above only matches LiteLLM in Anthropic proxy mode - # (api_mode == "anthropic_messages"). A LiteLLM deployment that - # exposes /v1/chat/completions instead matched no grant branch above - # and fell through to (False, False): no cache_control is injected, the - # system prompt goes on the wire as a plain string, and the provider - # serves zero cache hits — the entire prompt is re-billed at full price - # every turn. Same failure class already documented above for - # Qwen/DashScope. The endpoint supports Anthropic-style cache_control - # fine; only the provider detection missed it (#84506). - # - # Gated on the Claude family only: a Gemini/GPT/Qwen route through the - # same proxy must not receive markers — some strict OpenAI-wire relays - # reject the cache_control block format outright (cf. the DeepSeek / - # OpenCode exclusion below, #77217). - # - # Envelope layout (native_anthropic=False), matching every other - # OpenAI-wire grant in this function. The native inner-block layout - # writes a TOP-LEVEL msg["cache_control"] on role:tool and - # empty-content messages and relies on the Anthropic adapter to - # relocate it — but that adapter only runs for api_mode == - # "anthropic_messages" (agent/transports/anthropic.py), and the - # chat_completions transport performs no relocation. On this wire the - # native layout therefore (a) silently loses those breakpoints, spending - # 2 of the 4 available on markers the provider never sees, and (b) when - # LiteLLM relocates a top-level marker itself for an OpenRouter-backed - # Claude route, lands it on an empty text block — the HTTP 400 - # "text content blocks must contain" shape handled in - # agent/anthropic_adapter.py (#69512). - # - # Gated on chat_completions explicitly rather than `not - # is_anthropic_wire`: codex_responses / bedrock_converse are separate - # transports with their own marker handling and must not be swept in. + # LiteLLM fronting Claude on the OpenAI-compatible wire supports cache_control but matched + # no grant branch above (#84506). Claude-only: strict relays reject the block format for + # other models (#77217). Envelope layout: the native layout's top-level markers are only + # relocated by the anthropic_messages adapter and cause HTTP 400 via LiteLLM (#69512). + # Gated on chat_completions explicitly; codex_responses/bedrock_converse have their own handling. if _litellm_openai_wire: return True, False - # MiniMax on its Anthropic-compatible endpoint serves its own - # model family (MiniMax-M2.7, M2.5, M2.1, M2) with documented - # cache_control support (0.1× read pricing, 5-minute TTL). The - # blanket is_claude gate above excludes these — opt them in - # explicitly via provider id or host match so users on - # provider=minimax / minimax-cn (or custom endpoints pointing at - # api.minimax.io/anthropic / api.minimaxi.com/anthropic) get the - # same cost reduction as Claude traffic. MiniMax-M3 never reaches - # here — it is excluded before the native-Anthropic return above. - # Docs: https://platform.minimax.io/docs/api-reference/anthropic-api-compatible-cache + # MiniMax's own models (M2.x) on its Anthropic-compatible endpoint support cache_control; + # opt them in past the is_claude gate. M3 is excluded above. if is_anthropic_wire and is_minimax_route: return True, True - # Qwen/Alibaba on OpenCode (Zen/Go) and native DashScope: OpenAI-wire - # transport that accepts Anthropic-style cache_control markers and - # rewards them with real cache hits. Without this branch - # qwen3.6-plus on opencode-go reports 0% cached tokens and burns - # through the subscription on every turn. - # - # NOTE: DeepSeek models on OpenCode are intentionally excluded. - # OpenCode Zen's relay rejects the Anthropic-style content block - # format that cache markers produce (content becomes a block array - # instead of a plain string), causing HTTP 400 (#77217). - # Single source of truth for the family set and the qwen-model - # predicate — shared with the effective_cache_ttl clamp so the - # opt-in and the TTL clamp can never desync (#84733). + # Qwen/Alibaba on OpenCode and DashScope accept envelope cache_control on the OpenAI wire. + # DeepSeek on OpenCode is excluded: its relay 400s on block-array content (#77217). + # Family set/predicate shared with the effective_cache_ttl clamp (#84733). from agent.prompt_caching import ALIBABA_FAMILY_PROVIDERS, is_qwen_model model_is_qwen = is_qwen_model(model_lower) provider_is_alibaba_family = provider_lower in ALIBABA_FAMILY_PROVIDERS if provider_is_alibaba_family and model_is_qwen: - # Envelope layout (native_anthropic=False): markers on inner - # content parts, not top-level tool messages. Matches - # pi-mono's "alibaba" cacheControlFormat. + # Envelope layout (native_anthropic=False), matching pi-mono's "alibaba" cacheControlFormat. return True, False return False, False @@ -2777,23 +2056,11 @@ def _profile_for_base_url(base_url: str) -> Any | None: def create_openai_client(agent, client_kwargs: dict, *, reason: str, shared: bool) -> Any: from agent.auxiliary_client import _validate_base_url, _validate_proxy_env_urls from agent.ssl_verify import resolve_httpx_verify - # Treat client_kwargs as read-only. Callers pass agent._client_kwargs (or shallow - # copies of it) in; any in-place mutation leaks back into the stored dict and is - # reused on subsequent requests. #10933 hit this by injecting an httpx.Client - # transport that was torn down after the first request, so the next request - # wrapped a closed transport and raised "Cannot send a request, as the client - # has been closed" on every retry. The revert resolved that specific path; this - # copy locks the contract so future transport/keepalive work can't reintroduce - # the same class of bug. + # Treat client_kwargs as read-only: callers pass agent._client_kwargs, and in-place + # mutation leaks into later requests (#10933: a torn-down httpx transport got reused). client_kwargs = dict(client_kwargs) - # The MoA virtual provider has no real OpenAI wire endpoint - the facade - # *is* the client. Rebuilding a native OpenAI client while - # agent.provider == "moa" (client replacement, stream-retry pool cleanup, - # credential rotation, fallback+restore) drops the facade: the next primary - # call either raises a `_moa_prepared_request` TypeError (#78382) or, when - # _client_kwargs carry an unrelated relay base_url, leaks the request to a - # foreign gateway. Rebuild the facade instead (build_moa_facade also - # re-wires the reference relay, see #53802). + # The MoA virtual provider has no OpenAI wire endpoint; the facade *is* the client. + # Rebuild the facade, never a native client (#78382 TypeError, #53802 relay re-wire). if (getattr(agent, "provider", "") or "").strip().lower() == "moa": from agent.moa_loop import build_moa_facade return build_moa_facade(agent, getattr(agent, "model", None) or "default") @@ -2843,52 +2110,22 @@ def create_openai_client(agent, client_kwargs: dict, *, reason: str, shared: boo agent._client_log_context(), ) return client - # Inject TCP keepalives so the kernel detects dead provider connections - # instead of letting them sit silently in CLOSE-WAIT (#10324). Without - # this, a peer that drops mid-stream leaves the socket in a state where - # epoll_wait never fires, ``httpx`` read timeout may not trigger, and - # the agent hangs until manually killed. Probes after 30s idle, retry - # every 10s, give up after 3 → dead peer detected within ~60s. - # - # Safety against #10933: the ``client_kwargs = dict(client_kwargs)`` - # above means this injection only lands in the local per-call copy, - # never back into ``agent._client_kwargs``. Each ``_create_openai_client`` - # invocation therefore gets its OWN fresh ``httpx.Client`` whose - # lifetime is tied to the OpenAI client it is passed to. When the - # OpenAI client is closed (rebuild, teardown, credential rotation), - # the paired ``httpx.Client`` closes with it, and the next call - # constructs a fresh one — no stale closed transport can be reused. - # Tests in ``tests/run_agent/test_create_openai_client_reuse.py`` and - # ``tests/run_agent/test_sequential_chats_live.py`` pin this invariant. + # TCP keepalives so dead provider connections are detected (~60s) instead of hanging in + # CLOSE-WAIT (#10324). Injected into the local copy only (#10933), so each client gets its + # own httpx.Client; pinned by tests/run_agent/test_create_openai_client_reuse.py and + # tests/run_agent/test_sequential_chats_live.py. if "http_client" not in client_kwargs: keepalive_http = agent._build_keepalive_http_client( client_kwargs.get("base_url", ""), verify=httpx_verify, ) if keepalive_http is not None: client_kwargs["http_client"] = keepalive_http - # Delegate all rate-limit / 5xx retry to hermes's outer conversation loop, - # which honors Retry-After and applies adaptive/jittered backoff. The OpenAI - # SDK default (max_retries=2) uses its own 1-2s backoff that ignores - # Retry-After and double-retries inside our loop — the same deadlock the - # Anthropic clients hit (#26293). This is the single chokepoint every primary - # OpenAI/aggregator client passes through (init, switch_model, recovery, - # restore, request-scoped); auxiliary_client builds its own clients and keeps - # SDK retries because it is NOT wrapped by the conversation loop. + # Retries belong to the outer conversation loop (honors Retry-After); SDK retries would + # double-retry inside it (#26293). auxiliary_client keeps SDK retries as it isn't wrapped. client_kwargs.setdefault("max_retries", 0) - # Defense-in-depth: guarantee Copilot requests carry the integration - # headers regardless of which build path we came through. The primary - # header wiring lives in `_apply_client_headers_for_base_url`, but two - # rebuild paths (`primary_recovery`, `restore_primary` in this module) - # reconstruct the client purely from a `_primary_runtime` snapshot and do - # NOT re-run that wiring. If the snapshot's client_kwargs ever lacks - # `default_headers` (older snapshot, header-less resolver result), the - # client goes out WITHOUT `Copilot-Integration-Id: vscode-chat`; the - # Copilot server then routes it to the "copilot-language-server" integrator - # whose model allowlist omits enterprise-only models (claude-opus-4.8) → - # HTTP 400 model_not_available_for_integrator on every turn. This chokepoint - # is the single place every primary OpenAI client passes through, so filling - # missing Copilot headers here closes the whole class. We only ADD missing - # keys — never override headers a caller deliberately set. + # Defense-in-depth: primary_recovery/restore_primary rebuild from a _primary_runtime + # snapshot without re-running header wiring; missing Copilot-Integration-Id causes + # model_not_available_for_integrator 400s. Only ADD missing keys, never override. try: if base_url_host_matches(str(client_kwargs.get("base_url", "")), "githubcopilot.com"): from hermes_cli.models import copilot_default_headers @@ -2900,11 +2137,8 @@ def create_openai_client(agent, client_kwargs: dict, *, reason: str, shared: boo client_kwargs["default_headers"] = existing except Exception: _ra().logger.debug("Copilot default-header guard skipped", exc_info=True) - # OpenCode Free: the tier is served ANONYMOUSLY — any bearer the relay - # doesn't recognize (including placeholders) is a 401. Route every - # opencode-free client through the shared keyless header policy: an - # empty Authorization default_header overrides the SDK's - # "Bearer " so no credential ever reaches the wire. + # OpenCode Free is served anonymously: any unrecognized bearer is a 401, so an empty + # Authorization default_header overrides the SDK's "Bearer ". if agent.provider == "opencode-free": from hermes_cli.models import opencode_zen_free_headers @@ -2921,8 +2155,7 @@ def create_openai_client(agent, client_kwargs: dict, *, reason: str, shared: boo access_token=client_kwargs.get("api_key", ""), base_url=str(client_kwargs.get("base_url", "")), ) - # Uses the module-level `OpenAI` name, resolved lazily on first - # access via __getattr__ below. Tests patch via `run_agent.OpenAI`. + # Module-level `OpenAI` is resolved lazily via __getattr__; tests patch `run_agent.OpenAI`. client = _ra().OpenAI(**client_kwargs) _ra().logger.info( "OpenAI client created (%s, shared=%s) %s", @@ -2934,28 +2167,16 @@ def create_openai_client(agent, client_kwargs: dict, *, reason: str, shared: boo def _apply_switched_provider_request_overrides(agent, new_provider): - """Re-derive the switched-to provider's ``request_overrides`` onto a live agent. + """Re-derive the switched-to provider's ``request_overrides`` (custom_providers ``extra_body``) onto a live agent. - A ``custom_providers`` entry can carry an ``extra_body`` (e.g. - ``chat_template_kwargs`` to toggle a local model's thinking). The gateway - rebuild path carries this via ``request_overrides``; an *in-place* swap - (CLI / TUI ``/model``) must re-derive it for the switched-to provider, - otherwise the previous provider's ``extra_body`` lingers. - - The switched-to entry is matched by **provider key, base_url, and model** — - the same condition ``agent_init._merge_custom_provider_extra_body`` applies - at build time — via the shared ``_custom_provider_extra_body_for_agent`` - matcher. Matching by name alone would let a *different* model selected at the - same named endpoint inherit an ``extra_body`` configured for another model. - A stale ``extra_body`` is always cleared when the switched-to provider/model - resolves none; non-provider overrides (``service_tier`` / ``speed`` from - ``/fast``) are preserved. + Matches by provider key, base_url AND model (same rule as + ``agent_init._merge_custom_provider_extra_body``) so a different model at the + same endpoint never inherits another model's ``extra_body``. Stale + ``extra_body`` is cleared; ``service_tier`` / ``speed`` overrides are preserved. """ from agent.agent_init import _custom_provider_extra_body_for_agent - # Prefer the init-time cache (agent_init stores ``agent._custom_providers`` - # right where it runs its own _merge_custom_provider_extra_body); fall back - # to a fresh load only if a caller built the agent without it. + # Prefer the init-time cache (agent._custom_providers); reload only if absent. custom_providers = getattr(agent, "_custom_providers", None) if custom_providers is None: try: @@ -2987,45 +2208,31 @@ def switch_model( api_mode='', capabilities=None, ): - """Switch the model/provider in-place for a live agent. + """Switch the model/provider in-place for a live agent (rebuild clients, caching flags, compressor). - Called by the /model command handlers (CLI and gateway) after - ``model_switch.switch_model()`` has resolved credentials and - validated the model. This method performs the actual runtime - swap: rebuilding clients, updating caching flags, and refreshing - the context compressor. - - The implementation mirrors ``_try_activate_fallback()`` for the - client-swap logic but also updates ``_primary_runtime`` so the - change persists across turns (unlike fallback which is - turn-scoped). + Mirrors ``_try_activate_fallback()`` but also updates ``_primary_runtime`` so + the change persists across turns. """ from hermes_cli.providers import determine_api_mode from agent.native_compaction import resolve_native_compaction_capabilities old_model = agent.model old_provider = agent.provider + old_norm = (old_provider or "").strip().lower() + new_norm = (new_provider or "").strip().lower() - # ── Determine api_mode if not provided ── - # Pass model so dual-wire providers (Nous Portal anthropic/* → Messages) - # resolve correctly; without it determine_api_mode falls back to the - # openai_chat overlay default. + # Pass model so dual-wire providers (Nous Portal anthropic/* -> Messages) resolve correctly. if not api_mode: api_mode = determine_api_mode(new_provider, base_url, model=new_model) - normalized_new_provider = (new_provider or "").strip().lower() - if not base_url and normalized_new_provider == "openai": + if not base_url and new_norm == "openai": # An omitted URL means the provider's canonical direct endpoint. base_url = "https://api.openai.com/v1" - # Same-provider switches may omit base_url intentionally (for example, a - # direct caller refreshing credentials). Resolve capabilities from the - # endpoint that the normalization below will retain, not from the empty - # raw argument. + # Same-provider switches may omit base_url (e.g. credential refresh); resolve + # capabilities from the endpoint the normalization below retains. effective_base_url = base_url - if not effective_base_url and (old_provider or "").strip().lower() == ( - new_provider or "" - ).strip().lower(): + if not effective_base_url and old_norm == new_norm: effective_base_url = getattr(agent, "base_url", "") destination_capabilities = ( @@ -3035,15 +2242,12 @@ def switch_model( model=new_model, base_url=effective_base_url, provider=new_provider, - is_codex_backend=(new_provider or '').strip().lower() == 'openai-codex', + is_codex_backend=new_norm == 'openai-codex', ) ) - # Defense-in-depth: ensure OpenCode base_url doesn't carry a trailing - # /v1 into the anthropic_messages client, which would cause the SDK to - # hit /v1/v1/messages. `model_switch.switch_model()` already strips - # this, but we guard here so any direct callers (future code paths, - # tests) can't reintroduce the double-/v1 404 bug. + # Guard against a trailing /v1 on OpenCode base_url reaching the anthropic_messages + # client (double-/v1 404); model_switch already strips it, direct callers may not. from hermes_cli.models import opencode_provider_family if ( @@ -3054,17 +2258,9 @@ def switch_model( ): base_url = re.sub(r"/v1/?$", "", base_url) - # ── Snapshot all fields the swap+rebuild can mutate ── - # If the rebuild raises (bad API key, network error, build_anthropic_client - # failure, etc.) we restore these atomically so the agent isn't left with a - # new model/provider name paired with the OLD client — that mismatch causes - # HTTP 400s like "claude-sonnet-4-6 is not supported on openai-codex" on the - # next turn. Callers in cli.py / gateway/run.py / tui_gateway/server.py - # catch the re-raised exception and show the user a warning; without this - # rollback the warning is misleading because the swap partially succeeded. - # Use a sentinel so we can distinguish "attribute was unset" from - # "attribute was None" and skip the restore for genuinely-missing - # attributes (tests construct bare agents via __new__ without all fields). + # Snapshot every field the swap+rebuild mutates so a failed rebuild rolls back atomically + # (else new model name + OLD client -> 400s next turn). Sentinel distinguishes unset from + # None: tests build bare agents via __new__ without all fields. _MISSING = object() _snapshot = { name: getattr(agent, name, _MISSING) @@ -3085,12 +2281,9 @@ def switch_model( "runtime_capabilities", ) } - # _client_kwargs is a dict — snapshot a shallow copy so mutating the - # live dict doesn't poison the rollback target. + # Shallow-copy the dict so mutating the live one doesn't poison the rollback target. _snapshot["_client_kwargs"] = dict(getattr(agent, "_client_kwargs", {}) or {}) - # Snapshot the credential pool reference so a failed client rebuild can - # restore the original pool (issue #52727: pool reload is part of this - # switch and must be reversible on rollback). + # Pool reload is part of this switch and must be reversible on rollback (#52727). _snapshot["_credential_pool"] = getattr(agent, "_credential_pool", _MISSING) _snapshot["_credential_pool_entry_id"] = getattr( agent, "_credential_pool_entry_id", _MISSING @@ -3099,7 +2292,7 @@ def switch_model( def _restore_snapshot() -> None: for _name, _value in _snapshot.items(): if _value is _MISSING: - # Attribute did not exist before the swap — don't fabricate it. + # Attribute did not exist before the swap; don't fabricate it. continue try: setattr(agent, _name, _value) @@ -3107,62 +2300,37 @@ def switch_model( pass try: - # Clear the per-config context_length override so the new model's - # actual context window is resolved via get_model_context_length() - # instead of inheriting the stale value from the previous model. + # Clear the per-config override so the new model's context window is re-resolved. agent._config_context_length = None # ── Swap core runtime fields ── agent.model = new_model agent.provider = new_provider agent.requested_provider = new_provider - # Re-read reasoning_echo from config so the flag reflects the new - # primary model's setting (see _reasoning_echo_opt_in). + # Re-read reasoning_echo so the flag reflects the new primary model (see _reasoning_echo_opt_in). agent._reasoning_echo_flag = agent._read_reasoning_echo_from_config() - # Use the new base_url when provided. When it's empty AND the - # provider is actually changing, do NOT fall back to the current - # (old provider's) URL — that silently pairs the new provider label - # with the previous provider's endpoint (e.g. new_provider=minimax - # paired with the leftover api.githubcopilot.com URL), and every - # request after the switch 400s at the wrong host. This mismatched - # pair also gets snapshotted into _primary_runtime below, so it - # keeps re-applying on every subsequent turn until a full restart. - # Fail loud instead: the caller (model_switch.switch_model()) - # already resolves base_url for every real provider, so an empty - # value here means resolution failed upstream, not that the - # provider genuinely has none. Re-selecting the SAME provider with - # an empty base_url (e.g. a credential-only refresh) is still fine - # to keep the current URL. See #47828. - old_norm_provider = (old_provider or "").strip().lower() - new_norm_provider = (new_provider or "").strip().lower() + # Empty base_url while the provider changes means upstream resolution failed; falling + # back to the old provider's URL pairs the wrong host and persists via _primary_runtime + # (#47828). Fail loud. Same-provider re-select (credential refresh) may keep the URL. if base_url: agent.base_url = base_url - elif old_norm_provider != new_norm_provider: + elif old_norm != new_norm: raise ValueError( f"switch_model: no base_url resolved for provider " f"'{new_provider}' (switching from '{old_provider}'); " "refusing to keep the previous provider's endpoint" ) agent.api_mode = api_mode - # Invalidate transport cache — new api_mode may need a different transport + # New api_mode may need a different transport. if hasattr(agent, "_transport_cache"): agent._transport_cache.clear() if api_key: agent.api_key = api_key - # ── Reload credential pool for the new provider (issue #52727) ── - # Without this, ``recover_with_credential_pool`` sees a - # ``pool.provider != agent.provider`` mismatch and short-circuits, - # leaving the new provider with no rotation/recovery on 401/429 and - # burning the original pool's entries. Only reload when the provider - # actually changed (or the pool was missing) — re-selecting the same - # provider must not churn the pool reference. A reload failure is - # logged + swallowed: the switch itself must still complete. - old_norm = (old_provider or "").strip().lower() - new_norm = (new_provider or "").strip().lower() + # Reload the credential pool on provider change (#52727): a pool with a mismatched + # provider makes recover_with_credential_pool short-circuit. Reload failure is non-fatal. if old_norm != new_norm or getattr(agent, "_credential_pool", None) is None: - # A pool bound to the old provider is worse than no pool: the - # recovery guard rejects it and every later 401/429 skips rotation. + # A pool bound to the old provider is worse than none: the recovery guard rejects it. agent._credential_pool = None agent._credential_pool_entry_id = None try: @@ -3175,20 +2343,12 @@ def switch_model( new_provider, _pool_exc, ) # ── Build new client ── - if (new_provider or "").strip().lower() == "moa": + if new_norm == "moa": from agent.moa_loop import build_moa_facade - # The MoA virtual provider speaks only chat.completions via the - # MoAClient facade — the aggregator's real transport - # (codex_responses / anthropic_messages) is resolved and applied - # *inside* the reference/aggregator fan-out, never on the outer - # primary call. determine_api_mode("moa", ...) above may have left - # api_mode set to the aggregator's transport; if the conversation - # loop sees that, it dispatches client.responses.create (which the - # facade has no .responses for) and the call falls through to the - # moa://local placeholder → HTTP 404 → fallback to a reference - # model. Pin chat_completions here so the primary call always goes - # through MoAClient.chat.completions, matching agent_init.py. + # MoA speaks only chat.completions via the MoAClient facade; the aggregator's real + # transport is applied inside the fan-out. Pin api_mode so the loop never dispatches + # client.responses.create against the facade (matches agent_init.py). agent.api_mode = "chat_completions" agent.api_key = api_key or "moa-virtual-provider" agent.base_url = "moa://local" @@ -3200,22 +2360,19 @@ def switch_model( resolve_anthropic_token, _is_oauth_token, ) - # Only fall back to ANTHROPIC_TOKEN when the provider is actually Anthropic. - # Other anthropic_messages providers (MiniMax, Alibaba, etc.) must use their own - # API key — falling back would send Anthropic credentials to third-party endpoints. + # Only fall back to ANTHROPIC_TOKEN for native Anthropic; other anthropic_messages + # providers must never receive Anthropic credentials. _is_native_anthropic = new_provider == "anthropic" effective_key = (api_key or agent.api_key or resolve_anthropic_token() or "") if _is_native_anthropic else (api_key or agent.api_key or "") - # MiniMax OAuth: swap static string for a per-request callable token - # provider so the rebuilt client survives 15-min token expiry. See - # the matching block in agent_init.py for the full rationale. + # MiniMax OAuth: per-request callable token provider survives 15-min expiry + # (rationale in agent_init.py). if new_provider == "minimax-oauth" and isinstance(effective_key, str) and effective_key: try: from hermes_cli.auth import build_minimax_oauth_token_provider effective_key = build_minimax_oauth_token_provider() except Exception as _mm_exc: # noqa: BLE001 - import logging as _logging - _logging.getLogger(__name__).warning( + logger.warning( "MiniMax OAuth: failed to install per-request token provider " "on switch (%s); using static bearer.", _mm_exc, @@ -3245,10 +2402,8 @@ def switch_model( load_config_readonly, ) - # Read custom_providers from live config (not the init-time - # snapshot on ``agent._custom_providers``) so ssl_ca_cert / - # ssl_verify edits are honored when switching mid-session, - # matching the context-length reload below (#15779). + # Read live config, not agent._custom_providers, so mid-session ssl_ca_cert / + # ssl_verify edits are honored (#15779). apply_custom_provider_tls_to_client_kwargs( agent._client_kwargs, str(effective_base or ""), @@ -3259,10 +2414,8 @@ def switch_model( _sm_timeout = get_provider_request_timeout(agent.provider, agent.model) if _sm_timeout is not None: agent._client_kwargs["timeout"] = _sm_timeout - # Reapply provider-specific headers (e.g. OpenRouter HTTP-Referer, - # X-Title) that were lost when _client_kwargs was rebuilt from - # scratch. Without this, model switches clear attribution headers - # and OpenRouter logs show "Unknown" for subsequent requests. + # Reapply provider headers (OpenRouter HTTP-Referer/X-Title) lost when + # _client_kwargs was rebuilt; otherwise attribution shows "Unknown". agent._apply_client_headers_for_base_url(effective_base) agent.client = agent._create_openai_client( dict(agent._client_kwargs), @@ -3272,15 +2425,12 @@ def switch_model( sync_credential_pool_entry_id(agent) except Exception: - # Rollback every mutated field to the pre-swap snapshot so the agent - # is left consistent (old model + old provider + old client) and the - # caller's exception handler can surface a meaningful warning. The - # exception is re-raised; cli.py / gateway/run.py / tui_gateway catch - # it and print "Agent swap failed; change applied to next session". + # Roll back to the pre-swap snapshot so the agent stays consistent; callers + # (cli.py / gateway/run.py / tui_gateway) catch the re-raised exception. _restore_snapshot() raise - # ── LM Studio: preload before probing context length ── + # LM Studio: preload before probing context length. _sm_custom_providers = None try: from hermes_cli.config import ( @@ -3326,12 +2476,8 @@ def switch_model( else: _effective_context_length = _destination_context_intent - # ── Re-evaluate prompt caching ── - # Refresh the custom-provider snapshot from the config just loaded above - # so the per-model ``prompt_caching`` capability lookup sees the same - # live list the context-length resolution used — without this, a flag - # added to config.yaml after session start is invisible to a /model - # switch (the policy would read the stale init-time snapshot). + # Refresh the custom-provider snapshot from the config just loaded so the prompt_caching + # lookup sees flags added to config.yaml after session start. if _sm_custom_providers is not None: agent._custom_providers = _sm_custom_providers agent._use_prompt_caching, agent._use_native_cache_layout = ( @@ -3352,11 +2498,8 @@ def switch_model( _sm_custom_providers = get_compatible_custom_providers(load_config()) except Exception: _sm_custom_providers = None - # ``agent.api_key`` may be a callable (Azure Foundry Entra ID - # token provider). ``get_model_context_length`` expects a - # string for its live-probe paths; for Foundry the context - # length normally resolves via config or static catalogs and - # never hits a probe, but coerce to empty string defensively. + # agent.api_key may be a callable (Azure Foundry Entra ID); get_model_context_length + # expects a string for live probes, so coerce defensively. _ctx_api_key = agent.api_key if isinstance(agent.api_key, str) else "" try: new_context_length = get_model_context_length( @@ -3379,11 +2522,8 @@ def switch_model( _restore_snapshot() raise - # ── Re-resolve reasoning_config from per-model override ── - # The new model may have a different reasoning_effort override. Re-read - # config so the override takes effect immediately on /model switch — - # resolved through the shared chokepoint (per-model > global; YAML - # boolean False = disabled). + # Re-read the per-model reasoning_effort override so it applies immediately + # (per-model > global; YAML False = disabled). try: from hermes_constants import resolve_reasoning_config from hermes_cli.config import load_config as _sm_load_config @@ -3397,22 +2537,19 @@ def switch_model( except Exception as _reasoning_err: logger.debug("switch_model: could not re-resolve reasoning_config: %s", _reasoning_err) - # ── Invalidate cached system prompt so it rebuilds next turn ── + # Invalidate the cached system prompt so it rebuilds next turn. agent._cached_system_prompt = None # Publish the destination capability map only after every runtime setup # above has succeeded. Failed switches must leave the old map intact. agent.runtime_capabilities = destination_capabilities - # ── Reset the cross-turn stale-call circuit breaker (#58962) ── - # The breaker's error text tells the user to "switch models ... then - # retry"; without this reset the streak stays latched and the freshly - # selected (healthy) provider would keep short-circuiting before any - # stream is even attempted. + # Reset the cross-turn stale-call circuit breaker (#58962); otherwise the latched + # streak keeps short-circuiting the freshly selected healthy provider. from agent.chat_completion_helpers import _reset_stale_streak _reset_stale_streak(agent) - # ── Update _primary_runtime so the change persists across turns ── + # Update _primary_runtime so the change persists across turns. _cc = agent.context_compressor if hasattr(agent, "context_compressor") and agent.context_compressor else None agent._primary_runtime = { "model": agent.model, @@ -3426,10 +2563,8 @@ def switch_model( "use_native_cache_layout": agent._use_native_cache_layout, "reasoning_config": dict(agent.reasoning_config) if getattr(agent, "reasoning_config", None) else None, "reasoning_echo_flag": getattr(agent, "_reasoning_echo_flag", False), - # Request-level overrides (extra_body etc.) must travel with the - # switched-to identity; without this, a post-switch transport - # recovery or fallback restore would resurrect the PRE-switch - # overrides via the stale init-time snapshot (#75091 seam). + # Overrides must travel with the switched-to identity or a later recovery/restore + # resurrects PRE-switch overrides from the stale init snapshot (#75091). "request_overrides": dict(getattr(agent, "request_overrides", {}) or {}), "runtime_capabilities": dict(getattr(agent, "runtime_capabilities", {}) or {}), "compressor_model": getattr(_cc, "model", agent.model) if _cc else agent.model, @@ -3453,15 +2588,8 @@ def switch_model( agent._provider_fallback_route = None agent._fallback_index = 0 - # When the user deliberately swaps primary providers (e.g. openrouter - # → anthropic), drop any fallback entries that target the OLD primary - # or the NEW one. The chain was seeded from config at agent init for - # the original provider — without pruning, a failed turn on the new - # primary silently re-activates the provider the user just rejected, - # which is exactly what was reported during TUI v2 blitz testing - # ("switched to anthropic, tui keeps trying openrouter"). - old_norm = (old_provider or "").strip().lower() - new_norm = (new_provider or "").strip().lower() + # On a deliberate provider swap, prune fallback entries targeting the OLD or NEW primary; + # otherwise a failed turn silently re-activates the provider the user just rejected. fallback_chain = list(getattr(agent, "_fallback_chain", []) or []) if old_norm and new_norm and old_norm != new_norm: fallback_chain = [ @@ -3471,8 +2599,7 @@ def switch_model( agent._fallback_chain = fallback_chain agent._fallback_model = fallback_chain[0] if fallback_chain else None - # Apply the switched-to provider's request_overrides (custom_providers - # extra_body, e.g. chat_template_kwargs). See helper for rationale. + # Apply the switched-to provider's request_overrides (custom_providers extra_body). try: _apply_switched_provider_request_overrides(agent, new_provider) except Exception: @@ -3483,12 +2610,8 @@ def switch_model( old_model, old_provider, new_model, new_provider, ) - # ── Persist billing route to session DB ── - # The agent's _session_db / session_id may not be set in all contexts - # (tests, bare agents without a session DB, etc.). This ensures the - # dashboard Model cards show the actual provider after a mid-session - # /model switch instead of the stale session-creation provider. - # See #48248 for the full bug description. + # Persist billing route so dashboard Model cards show the post-switch provider (#48248). + # _session_db / session_id may be unset (tests, bare agents). _session_db = getattr(agent, "_session_db", None) _session_id = getattr(agent, "session_id", None) if _session_db is not None and _session_id: @@ -3512,12 +2635,18 @@ def invoke_tool(agent, function_name: str, function_args: dict, effective_task_i skip_tool_request_middleware: bool = False, tool_request_middleware_trace: Optional[List[Dict[str, Any]]] = None, skip_tool_execution_middleware: bool = False) -> str: - """Invoke a single tool and return the result string. No display logic. + """Invoke a single tool and return the result string; no display logic. - Handles both agent-level tools (todo, memory, etc.) and registry-dispatched - tools. Used by the concurrent execution path; the sequential path retains - its own inline invocation for backward-compatible display handling. + Handles agent-level and registry-dispatched tools. Used by the concurrent + path; the sequential path keeps its own inline invocation for display. """ + from agent.inline_tool_executors import ( + InlineToolContext, + emit_terminal_post_tool_call, + resolve_invoke_tool_executor, + tool_hook_ids, + ) + if not isinstance(function_args, dict): function_args = {} @@ -3529,11 +2658,7 @@ def invoke_tool(agent, function_name: str, function_args: dict, effective_task_i _tool_request_mw = apply_tool_request_middleware( function_name, function_args, - task_id=effective_task_id or "", - session_id=getattr(agent, "session_id", "") or "", - tool_call_id=tool_call_id or "", - turn_id=getattr(agent, "_current_turn_id", "") or "", - api_request_id=getattr(agent, "_current_api_request_id", "") or "", + **tool_hook_ids(agent, effective_task_id, tool_call_id), ) function_args = _tool_request_mw.payload _tool_middleware_trace = _tool_request_mw.trace @@ -3559,222 +2684,45 @@ def invoke_tool(agent, function_name: str, function_args: dict, effective_task_i block_message = None if block_message is not None: result = json.dumps({"error": block_message}, ensure_ascii=False) - try: - from model_tools import _emit_post_tool_call_hook - _emit_post_tool_call_hook( - function_name=function_name, - function_args=function_args, - result=result, - task_id=effective_task_id or "", - session_id=getattr(agent, "session_id", "") or "", - tool_call_id=tool_call_id or "", - turn_id=getattr(agent, "_current_turn_id", "") or "", - api_request_id=getattr(agent, "_current_api_request_id", "") or "", - status="blocked", - error_type="plugin_block", - error_message=block_message, - middleware_trace=list(_tool_middleware_trace), - ) - except Exception: - pass + emit_terminal_post_tool_call( + agent, + function_name=function_name, + function_args=function_args, + result=result, + effective_task_id=effective_task_id, + tool_call_id=tool_call_id, + status="blocked", + error_type="plugin_block", + error_message=block_message, + middleware_trace=_tool_middleware_trace, + ) return result tool_start_time = time.monotonic() def _finish_agent_tool(result: Any, observed_args: Optional[dict] = None) -> Any: - hook_args = observed_args if isinstance(observed_args, dict) else function_args - try: - from model_tools import _emit_post_tool_call_hook - _emit_post_tool_call_hook( - function_name=function_name, - function_args=hook_args, - result=result, - task_id=effective_task_id or "", - session_id=getattr(agent, "session_id", "") or "", - tool_call_id=tool_call_id or "", - turn_id=getattr(agent, "_current_turn_id", "") or "", - api_request_id=getattr(agent, "_current_api_request_id", "") or "", - duration_ms=int((time.monotonic() - tool_start_time) * 1000), - middleware_trace=list(_tool_middleware_trace), - ) - except Exception: - pass + emit_terminal_post_tool_call( + agent, + function_name=function_name, + function_args=observed_args if isinstance(observed_args, dict) else function_args, + result=result, + effective_task_id=effective_task_id, + tool_call_id=tool_call_id, + duration_ms=int((time.monotonic() - tool_start_time) * 1000), + middleware_trace=_tool_middleware_trace, + ) return result - if function_name == "todo_list": + inline_executor = resolve_invoke_tool_executor(agent, function_name) + if inline_executor is not None: + inline_ctx = InlineToolContext( + effective_task_id=effective_task_id, + tool_call_id=tool_call_id, + messages=messages, + ) + def _execute(next_args: dict) -> Any: - from tools.todo_tool import todo_tool as _todo_tool - return _finish_agent_tool( - _todo_tool( - todos=next_args.get("todos"), - merge=next_args.get("merge", False), - store=agent._todo_store, - ), - next_args, - ) - elif function_name == "session_search": - def _execute(next_args: dict) -> Any: - session_db = agent._get_session_db_for_recall() - if not session_db: - from hermes_state import format_session_db_unavailable - return _finish_agent_tool(json.dumps({"success": False, "error": format_session_db_unavailable()}), next_args) - from tools.session_search_tool import session_search as _session_search - return _finish_agent_tool( - _session_search( - query=next_args.get("query", ""), - role_filter=next_args.get("role_filter"), - limit=next_args.get("limit", 3), - session_id=next_args.get("session_id"), - around_message_id=next_args.get("around_message_id"), - window=next_args.get("window", 5), - sort=next_args.get("sort"), - detail=next_args.get("detail", "adaptive"), - db=session_db, - current_session_id=agent.session_id, - ), - next_args, - ) - elif function_name == "memory": - def _execute(next_args: dict) -> Any: - target = next_args.get("target", "memory") - operations = next_args.get("operations") - from tools.memory_tool import memory_tool as _memory_tool - result = _memory_tool( - action=next_args.get("action"), - target=target, - content=next_args.get("content"), - old_text=next_args.get("old_text"), - operations=operations, - store=agent._memory_store, - ) - # Mirror successful built-in memory writes to external providers. - # All gating/op-expansion lives behind the manager interface - # (MemoryManager.notify_memory_tool_write). - if agent._memory_manager: - agent._memory_manager.notify_memory_tool_write( - result, - next_args, - build_metadata=lambda: agent._build_memory_write_metadata( - task_id=effective_task_id, - tool_call_id=tool_call_id, - ), - ) - return _finish_agent_tool(result, next_args) - elif agent._memory_manager and agent._memory_manager.has_tool(function_name): - def _execute(next_args: dict) -> Any: - return _finish_agent_tool(agent._memory_manager.handle_tool_call(function_name, next_args), next_args) - elif function_name == "clarify": - def _execute(next_args: dict) -> Any: - from tools.clarify_tool import clarify_tool as _clarify_tool - return _finish_agent_tool( - _clarify_tool( - question=next_args.get("question", ""), - choices=next_args.get("choices"), - multi_select=next_args.get("multi_select", False), - questions=next_args.get("questions"), - callback=agent.clarify_callback, - ), - next_args, - ) - elif function_name == "read_terminal": - def _execute(next_args: dict) -> Any: - from tools.read_terminal_tool import read_terminal_tool as _read_terminal_tool - return _finish_agent_tool( - _read_terminal_tool( - start_line=next_args.get("start_line"), - count=next_args.get("count"), - callback=getattr(agent, "read_terminal_callback", None), - ), - next_args, - ) - elif function_name == "desktop_preview": - def _execute(next_args: dict) -> Any: - # action=read needs the GUI callback (agent-level); open/close go - # through the registry handler like any other tool. - if (next_args.get("action") or "").strip() == "read": - from tools.read_preview_tool import read_preview_tool as _read_preview_tool - return _finish_agent_tool( - _read_preview_tool( - start=next_args.get("start"), - count=next_args.get("count"), - callback=getattr(agent, "read_preview_callback", None), - ), - next_args, - ) - from tools.preview_tool import _handle_preview - return _finish_agent_tool(_handle_preview(next_args), next_args) - elif function_name == "drive_preview": - def _execute(next_args: dict) -> Any: - from tools.drive_preview_tool import drive_preview_tool as _drive_preview_tool - return _finish_agent_tool( - _drive_preview_tool( - action=next_args.get("action", ""), - ref=next_args.get("ref"), - selector=next_args.get("selector"), - text=next_args.get("text"), - key=next_args.get("key"), - submit=next_args.get("submit"), - amount=next_args.get("amount"), - to=next_args.get("to"), - limit=next_args.get("max"), - callback=getattr(agent, "drive_preview_callback", None), - ), - next_args, - ) - elif function_name == "annotate_preview": - def _execute(next_args: dict) -> Any: - from tools.annotate_preview_tool import annotate_preview_tool as _annotate_preview_tool - return _finish_agent_tool( - _annotate_preview_tool( - action=next_args.get("action", "add"), - ref=next_args.get("ref"), - selector=next_args.get("selector"), - label=next_args.get("label"), - callback=getattr(agent, "drive_preview_callback", None), - ), - next_args, - ) - elif function_name == "read_window_below": - def _execute(next_args: dict) -> Any: - from tools.read_window_tool import read_window_below_tool as _read_window_below_tool - return _finish_agent_tool( - _read_window_below_tool( - callback=getattr(agent, "read_window_below_callback", None), - ), - next_args, - ) - elif function_name == "gui_tour": - def _execute(next_args: dict) -> Any: - from tools.tour_tool import tour_tool as _tour_tool - return _finish_agent_tool( - _tour_tool( - action=next_args.get("action", ""), - surface=next_args.get("surface"), - selector=next_args.get("selector"), - title=next_args.get("title"), - text=next_args.get("text"), - side=next_args.get("side"), - steps=next_args.get("steps"), - step_index=next_args.get("step_index"), - callback=getattr(agent, "tour_callback", None), - ), - next_args, - ) - elif function_name == "setup_mcp": - def _execute(next_args: dict) -> Any: - from tools.setup_mcp_tool import setup_mcp_tool as _setup_mcp_tool - return _finish_agent_tool( - _setup_mcp_tool( - server=next_args.get("server", ""), - action=next_args.get("action", "install"), - reason=next_args.get("reason", ""), - callback=getattr(agent, "setup_mcp_callback", None), - ), - next_args, - ) - elif function_name == "delegate_task": - def _execute(next_args: dict) -> Any: - return _finish_agent_tool(agent._dispatch_delegate_task(next_args), next_args) + return _finish_agent_tool(inline_executor(agent, next_args, inline_ctx), next_args) else: def _execute(next_args: dict) -> Any: dispatch_kwargs = dict( @@ -3808,35 +2756,16 @@ def invoke_tool(agent, function_name: str, function_args: dict, effective_task_i function_args, lambda next_args: _execute(next_args if isinstance(next_args, dict) else function_args), original_args=function_args, - task_id=effective_task_id or "", - session_id=getattr(agent, "session_id", "") or "", - tool_call_id=tool_call_id or "", - turn_id=getattr(agent, "_current_turn_id", "") or "", - api_request_id=getattr(agent, "_current_api_request_id", "") or "", + **tool_hook_ids(agent, effective_task_id, tool_call_id), ) def repair_tool_call(agent, tool_name: str) -> str | None: - """Attempt to repair a mismatched tool name before aborting. + """Repair a mismatched tool name (case, separators, CamelCase, ``_tool`` suffixes, then fuzzy match) before aborting. - Models sometimes emit variants of a tool name that differ only - in casing, separators, or class-like suffixes. Normalize - aggressively before falling back to fuzzy match: - - 1. Lowercase direct match. - 2. Lowercase + hyphens/spaces -> underscores. - 3. CamelCase -> snake_case (TodoTool -> todo_tool). - 4. Strip trailing ``_tool`` / ``-tool`` / ``tool`` suffix that - Claude-style models sometimes tack on (TodoTool_tool -> - TodoTool -> Todo -> todo). Applied twice so double-tacked - suffixes like ``TodoTool_tool`` reduce all the way. - 5. Fuzzy match (difflib, cutoff=0.7). - - See #14784 for the original reports (TodoTool_tool, Patch_tool, - BrowserClick_tool were all returning "Unknown tool" before). - - Returns the repaired name if found in valid_tool_names, else None. + Suffix stripping is applied twice so ``TodoTool_tool`` reduces fully (#14784). + Returns the repaired name if in valid_tool_names, else None. """ import re from difflib import get_close_matches @@ -3844,20 +2773,10 @@ def repair_tool_call(agent, tool_name: str) -> str | None: if not tool_name: return None - # VolcEngine api/plan workaround (issue #33007): the endpoint's - # protocol-translation layer occasionally leaks raw XML attribute - # fragments into tool_use.name, e.g. - # `terminal" parameter="command" string="true` - # `execute_code" parameter="code" string="true` - # `session_search" parameter="session_id" string="true` - # We trim at the first unambiguous XML/quote character so the rest - # of the repair pipeline (lowercase / snake_case / fuzzy match) - # can resolve the cleaned name to a real tool. - # - # Crucially we DO NOT split on whitespace: legitimate inputs like - # "write file" must keep flowing through ``_norm`` -> ``write_file`` - # (covered by test_space_to_underscore in - # tests/run_agent/test_repair_tool_call_name.py). + # VolcEngine api/plan (#33007) leaks XML attribute fragments into tool_use.name + # (`terminal" parameter="command" ...`); trim at the first quote/angle bracket. + # Do NOT split on whitespace: "write file" must reach ``_norm`` -> ``write_file`` + # (test_space_to_underscore in tests/run_agent/test_repair_tool_call_name.py). for _xml_sep in ('"', "'", "<", ">"): _idx = tool_name.find(_xml_sep) if _idx > 0: @@ -3878,7 +2797,7 @@ def repair_tool_call(agent, tool_name: str) -> str | None: return s[: -len(suffix)].rstrip("_-") return None - # Cheap fast-paths first — these cover the common case. + # Cheap fast-paths first. lowered = tool_name.lower() if lowered in agent.valid_tool_names: return lowered @@ -3886,9 +2805,8 @@ def repair_tool_call(agent, tool_name: str) -> str | None: if normalized in agent.valid_tool_names: return normalized - # Build the full candidate set for class-like emissions. cands: set[str] = {tool_name, lowered, normalized, _camel_snake(tool_name)} - # Strip trailing tool-suffix up to twice — TodoTool_tool needs it. + # Strip trailing tool-suffix up to twice (TodoTool_tool needs it). for _ in range(2): extra: set[str] = set() for c in cands: @@ -3903,7 +2821,6 @@ def repair_tool_call(agent, tool_name: str) -> str | None: if c and c in agent.valid_tool_names: return c - # Fuzzy match as last resort. matches = get_close_matches(lowered, agent.valid_tool_names, n=1, cutoff=0.7) if matches: return matches[0] @@ -3912,49 +2829,36 @@ def repair_tool_call(agent, tool_name: str) -> str | None: def _tool_call_id_variants(tc: Any) -> set: - """Return every id a tool result might legitimately match this tool_call on. + """Return every id a tool result might match this tool_call on. - Thin forwarder — the policy owner is - ``agent.message_sanitization.tool_call_id_variants`` (handles ``id``, - ``call_id``, ``response_item_id``, and composite ``call|item`` spellings). - Kept for backward compatibility with existing importers. + Thin backward-compatible forwarder; policy owner is + ``agent.message_sanitization.tool_call_id_variants``. """ return set(tool_call_id_variants(tc)) -# Placeholder substituted for an empty non-final message that would otherwise -# make the provider reject the whole request. Kept identical to the stub- -# creation placeholder in chat_completion_helpers so a healed transcript reads -# consistently whether the empty turn was caught at write time or send time. +# Placeholder for an empty non-final message the provider would reject. Kept identical to +# the stub placeholder in chat_completion_helpers so healed transcripts read consistently. _INTERRUPTED_PLACEHOLDER = "[response interrupted]" -# Repeated heals of the same poisoned transcript used to WARNING on every -# send (#96870). Escalate once per session window, then stay quiet. -# ``_EMPTY_HEAL_ESCALATE_AFTER`` is the built-in default; deployments tune it -# via ``agent.sanitizer_heal_escalation_threshold`` in config.yaml (<= 0 -# disables escalation entirely — WARNINGs still fire per window). +# Escalate repeated heals once per session window, then stay quiet (#96870). Default +# threshold; tunable via ``agent.sanitizer_heal_escalation_threshold`` (<= 0 disables). _EMPTY_HEAL_ESCALATE_AFTER = 3 _EMPTY_HEAL_WINDOW_S = 600.0 _empty_heal_log_state: Dict[str, Dict[str, Any]] = {} _empty_heal_log_lock = threading.Lock() -# Session keys that already received the one-time user notice. Separate from -# the windowed log state so a new 10-minute window never re-notifies: the -# user is told ONCE per session, ever (#96870 — out-of-band, delivery -# channel only, never injected into conversation context). +# Sessions already given the one-time user notice; separate from the windowed log state +# so the user is told ONCE per session (#96870, out-of-band, never in conversation context). _empty_heal_user_notified: set = set() -# One-shot pending notices keyed by session, drained by the conversation -# loop through ``consume_pending_sanitizer_heal_notice`` and delivered via -# the status/warning callback (the normal delivery channel). +# One-shot pending notices keyed by session, drained by the conversation loop via +# ``consume_pending_sanitizer_heal_notice`` and delivered via the status/warning callback. _empty_heal_pending_notice: Dict[str, str] = {} def _msg_has_payload(msg: Dict[str, Any]) -> bool: - """True if ``msg`` carries anything the API treats as non-empty content. + """True if ``msg`` carries anything the API treats as non-empty content (text, multimodal blocks, tool_calls, tool_call_id, reasoning). - Covers string content, non-empty multimodal content lists, tool_calls, - tool_call_id linkage (tool results), and reasoning payloads. Mirrors the - emptiness checks used by ``AIAgent._is_thinking_only_assistant`` but is - role-agnostic so it can vet user/assistant/tool turns uniformly. + Role-agnostic counterpart of ``AIAgent._is_thinking_only_assistant``. """ content = msg.get("content") if isinstance(content, str): @@ -3963,8 +2867,7 @@ def _msg_has_payload(msg: Dict[str, Any]) -> bool: elif isinstance(content, list): for block in content: if isinstance(block, dict): - # any typed block (text/image/tool_use/document/...) counts, - # as long as a text block is not itself blank + # any typed block counts, as long as a text block is not itself blank if block.get("type") == "text": if isinstance(block.get("text"), str) and block["text"].strip(): return True @@ -3981,30 +2884,19 @@ def _msg_has_payload(msg: Dict[str, Any]) -> bool: return True if msg.get("reasoning") or msg.get("reasoning_details"): return True - # Codex Responses item carriers: a commentary-phase assistant turn - # persists with content:"" by DESIGN — its text lives in - # ``codex_message_items`` (delivered via the interim callback) and the - # structured items are replayed for prefix-cache hits. Same for - # ``codex_reasoning_items``. These turns are never wire-empty on any - # api_mode: the codex transport replays the items, and the - # chat-completions transport strips the carriers only after this repair - # pass has already run. Treat them as payload so the repair never - # rewrites a designed-empty codex turn (July 2026: a write-time pad that - # ignored this broke codex commentary replay in CI). - if msg.get("codex_message_items") or msg.get("codex_reasoning_items"): - return True - return False + # Codex Responses item carriers persist with content:"" by design (text lives in + # codex_message_items / codex_reasoning_items and is replayed); treat as payload so + # the repair never rewrites a designed-empty codex turn. + return bool(msg.get("codex_message_items") or msg.get("codex_reasoning_items")) def fill_empty_non_final_wire_payload( msg: Dict[str, Any], *, is_final: bool ) -> bool: - """Write the interrupted placeholder onto an empty non-final wire copy. + """Write the interrupted placeholder onto an empty non-final wire copy; returns True when filled. - Used by the send-time projection so ``repair_empty_non_final_messages`` - does not re-heal the same row on every call (#88955 hidden placeholders, - #96870 stream-death / host-fed empties). Pass the per-call copy only — - durable history must not be mutated. Returns True when *msg* was filled. + Pass the per-call copy only; durable history must not be mutated + (#88955, #96870). """ if is_final or not isinstance(msg, dict): return False @@ -4026,12 +2918,7 @@ def _session_id_for_heal_log() -> str: def _heal_escalation_threshold() -> int: - """Resolve the escalation threshold: config override, else the default. - - ``agent.sanitizer_heal_escalation_threshold`` in config.yaml. Fail-safe: - any read error falls back to the module default so the sanitiser can - never be broken by a bad config file. - """ + """Escalation threshold from ``agent.sanitizer_heal_escalation_threshold``, else the module default (fail-safe on any read error).""" try: from hermes_cli.config import load_config_readonly @@ -4046,14 +2933,10 @@ def _heal_escalation_threshold() -> int: def consume_pending_sanitizer_heal_notice() -> Optional[str]: - """Drain the one-time user notice for the current session, if any. + """Drain the one-time user notice for the current session, if any (at most one per session lifetime). - Called by the conversation loop right after the pre-send sanitizer pass; - the returned text is delivered through the status/warning callback (the - normal out-of-band delivery channel: gateway status message, CLI stderr - print). It is NEVER appended to the conversation context, so prompt - caching and role alternation are untouched. Returns at most one notice - per session for its whole lifetime. + Delivered through the status/warning callback, NEVER appended to the + conversation context. """ key = _session_id_for_heal_log() or "-" with _empty_heal_log_lock: @@ -4061,13 +2944,10 @@ def consume_pending_sanitizer_heal_notice() -> Optional[str]: def get_sanitizer_heal_stats() -> Dict[str, Dict[str, Any]]: - """Read-only snapshot of per-session sanitiser heal counters. + """Read-only snapshot of per-session sanitiser heal counters for diagnostics. - Surfaced by diagnostics (``hermes doctor`` / debug share callers) so - repeated silent repairs are visible outside errors.log. Keys are session - ids; values carry ``heal_events`` (sanitizer invocations that healed at - least one message), ``messages_healed`` (total substituted turns) and - ``escalated`` (whether the ERROR + user notice fired). + Keyed by session id; values carry ``heal_events``, ``messages_healed`` and + ``escalated``. """ with _empty_heal_log_lock: return { @@ -4081,14 +2961,10 @@ def get_sanitizer_heal_stats() -> Dict[str, Dict[str, Any]]: def _log_empty_non_final_heal(healed: int) -> None: - """WARNING on the first heals in a window; one ERROR at the threshold. + """WARNING on the first heals in a window, one ERROR at the threshold, then silent (#96870). - Further heals in the same session window stay silent so a poisoned - transcript cannot flood ``errors.log`` (dozens of identical WARNINGs - per hour with no user-visible signal — #96870). At the threshold the - escalation also queues a ONE-TIME out-of-band user notice (drained by - ``consume_pending_sanitizer_heal_notice``) pointing at ``/debug share`` - / ``hermes doctor`` — once per session, never re-armed by a new window. + The threshold also queues a ONE-TIME out-of-band user notice (drained by + ``consume_pending_sanitizer_heal_notice``); never re-armed by a new window. """ key = _session_id_for_heal_log() or "-" threshold = _heal_escalation_threshold() @@ -4160,34 +3036,12 @@ def _log_empty_non_final_heal(healed: int) -> None: def repair_empty_non_final_messages( messages: List[Dict[str, Any]], ) -> List[Dict[str, Any]]: - """Heal empty-content non-final messages before they reach the provider. + """Substitute a placeholder for empty-content non-final messages on the per-call copy. - Root-cause context: a stream that dies with 0 recovered characters (peer - reset, stall-kill) could persist an assistant turn with ``content=None`` - and no tool_calls. The Anthropic message schema — and the litellm/Bedrock - proxies in front of it — reject ANY request whose transcript contains an - empty non-final message: - - "all messages must have non-empty content except for the optional - final assistant message" (HTTP 400 INVALID_REQUEST_BODY) - - Once such a message lands mid-transcript it poisons EVERY subsequent turn - of that session until it scrolls out of context. The write-time guard in - ``chat_completion_helpers`` stops NEW stubs, but sessions already carrying - one (persisted before the guard, or fed in from a host history) stay stuck - and previously needed a manual DB edit + gateway restart to recover. - - This pass is the self-healing counterpart: it runs unconditionally on the - per-call ``api_messages`` copy, so a poisoned transcript repairs itself - IN MEMORY on the very next send — no restart, no DB surgery. The final - message is left untouched (an empty final assistant turn is legal). The - stored conversation history is never mutated; only the wire copy is - repaired, so the UI/session trace stays faithful. - - Repair strategy is substitution, not deletion: dropping a mid-transcript - turn can break role alternation and tool-call pairing, whereas an honest - minimal placeholder keeps the sequence intact and reads correctly as an - interrupted turn on replay. + Anthropic/litellm/Bedrock 400 on any empty non-final message, and a + persisted stub poisons every later turn; repairing the wire copy heals + the session in memory. Substitution (not deletion) keeps role alternation + and tool-call pairing intact. The final message is left untouched. """ if not messages or len(messages) < 2: return messages @@ -4199,8 +3053,7 @@ def repair_empty_non_final_messages( if ( idx != last_idx and isinstance(msg, dict) - # tool results are validated by their own orphan/pairing pass; an - # empty tool result is a separate (and rarer) concern. + # Tool results are checked by their own pairing pass; empty ones are a separate concern. and msg.get("role") in ("assistant", "user") and not _msg_has_payload(msg) ): @@ -4219,37 +3072,15 @@ def repair_empty_non_final_messages( def _classify_tool_call_orphans(messages: List[Dict[str, Any]]): - """Classify orphaned tool-call / tool-result pairs in *messages*. + """Classify orphaned tool-call / tool-result pairs; single source of truth for GLOBAL orphan detection. - Returns a 4-tuple ``(surviving_call_ids, result_call_ids, - orphaned_results, missing_tool_calls)``: - - - ``surviving_call_ids``: every id variant carried by any assistant - ``tool_calls`` entry. A tool_call may carry SEVERAL equivalent id - spellings (``id`` fc_..., ``call_id`` call_..., ``response_item_id``, - or a composite ``call|item`` bridge id) — register EVERY variant so a - result matching any of them survives (#55626, #63000). - - ``result_call_ids``: every id variant referenced by a ``tool`` result. - - ``orphaned_results``: the actual ``tool`` message dicts whose complete - alias set matches no assistant call (compare by identity — ``id(msg)`` - — when filtering, since dicts are unhashable). - - ``missing_tool_calls``: the actual tool_call entries with no matching - result on ANY alias (after orphaned results are excluded). - - This is the single source of truth for GLOBAL orphan *detection* (does - a matching id exist anywhere in the transcript). Its remaining consumer - is the context compressor's ``_sanitize_tool_pairs`` (strip orphans from - the durable history); the id-resolution and variant-expansion rules live - here so they can never drift between call sites again (#58357). - - ``sanitize_api_messages`` no longer uses the global check: strict - positional providers (DeepSeek v4, Kimi) reject a call whose result is - not in the IMMEDIATELY-following tool run even when a matching id exists - elsewhere, so its pairing pass walks the transcript positionally instead - (#94704) — a strictly stronger invariant that subsumes the global one at - that site. Both share the same ``tool_call_id_variants`` / - ``tool_result_id_variants`` alias policy, which is the part that must - not fork. + Returns ``(surviving_call_ids, result_call_ids, orphaned_results, + missing_tool_calls)``. Every id variant of a tool_call (``id``, + ``call_id``, ``response_item_id``, composite bridge) is registered so a + result matching any alias survives (#55626, #63000, #58357). + ``orphaned_results`` are the actual dicts (filter by ``id(msg)``). + ``sanitize_api_messages`` pairs positionally instead (#94704) but shares + the ``tool_call_id_variants`` / ``tool_result_id_variants`` alias policy. """ assistant_call_variants: List[tuple[Any, frozenset[str]]] = [] surviving_call_ids: set[str] = set() @@ -4291,12 +3122,7 @@ def _classify_tool_call_orphans(messages: List[Dict[str, Any]]): def sanitize_api_messages(messages: List[Dict[str, Any]]) -> List[Dict[str, Any]]: - """Fix orphaned tool_call / tool_result pairs before every LLM call. - - Runs unconditionally — not gated on whether the context compressor - is present — so orphans from session loading or manual message - manipulation are always caught. - """ + """Fix orphaned tool_call / tool_result pairs before every LLM call; runs unconditionally (not gated on the compressor).""" # --- Role allowlist: drop messages with roles the API won't accept --- filtered = [] for msg in messages: @@ -4311,29 +3137,14 @@ def sanitize_api_messages(messages: List[Dict[str, Any]]) -> List[Dict[str, Any] messages = filtered # --- Heal empty-content non-final messages (self-recovery) --- - # A dead stream can leave an empty assistant stub (or an empty user turn) - # mid-transcript; the provider then 400s EVERY subsequent request until it - # scrolls out. Repair it here, on the per-call copy, so a poisoned session - # recovers itself in memory on the next send — no restart, no DB edit. - # Done first so a substituted turn participates normally in the tool-pair - # and dedup passes below. + # A dead stream can leave an empty stub mid-transcript that 400s every later request; + # repair the per-call copy so the session heals in memory. Done first so the substituted + # turn participates in the tool-pair and dedup passes below. messages = repair_empty_non_final_messages(messages) # --- Drop empty / malformed tool_calls arrays on assistant messages --- - # An assistant message carrying ``tool_calls: []`` (an empty array) — or a - # non-list value under the key — is semantically identical to an assistant - # message with no tool calls, but strict OpenAI-compatible providers reject - # the empty array outright: DeepSeek v4 returns HTTP 400 "Invalid - # 'messages[N].tool_calls': empty array. Expected an array with minimum - # length 1, but got an empty array instead." (#58755, follow-up to #56980). - # Empty arrays reach here from session resume, host-fed histories, or the - # consecutive-assistant merge in ``repair_message_sequence`` (which - # preserves a pre-existing ``[]`` on the surviving turn). This is the final - # pre-API chokepoint, so normalize defensively — and, per the #56980 - # review, do it HERE on the per-call copy rather than in - # ``repair_message_sequence``, which would destructively rewrite the - # persisted trajectory. Shallow-copy the message before dropping the key so - # stored history (and prompt caching) stays byte-stable. + # Strict providers 400 on ``tool_calls: []`` (#58755, #56980). Normalize on the + # per-call copy (shallow-copy) so persisted history stays byte-stable. normalized: List[Dict[str, Any]] = [] dropped_empty_tool_calls = 0 for msg in messages: @@ -4355,21 +3166,8 @@ def sanitize_api_messages(messages: List[Dict[str, Any]]) -> List[Dict[str, Any] ) # --- Repair tool_calls whose function.name is empty/missing --- - # Some providers (and partially-streamed responses) emit a tool_call with - # id="call_xxx" but function.name="". Downstream Responses-API adapters - # silently DROP such function_call items while still emitting the matching - # function_call_output, producing the gateway's HTTP 400 - # "No tool call found for function call output with call_id ...". - # - # We do NOT drop the call: hermes' own dispatch loop intentionally keeps an - # empty-name call paired with a synthesized anti-priming tool result - # ("tool name was empty", see #47967) so weak models self-correct instead of - # being fed the full tool catalog. Dropping the call here would (a) orphan - # that result and strip the anti-priming signal, and (b) still leave any - # provider-side orphan. Instead, rename the blank name to a non-empty - # sentinel so the call and its result stay PAIRED — the adapter no longer - # drops the function_call, so there is no orphaned output and no 400, while - # the result content the model needs is preserved. + # Rename to a sentinel instead of dropping: the dispatch loop keeps empty-name calls + # paired with an anti-priming result (#47967), and Responses adapters drop nameless calls (400). _EMPTY_NAME_SENTINEL = "invalid_tool_call" for msg in messages: if msg.get("role") != "assistant": @@ -4403,19 +3201,8 @@ def sanitize_api_messages(messages: List[Dict[str, Any]]) -> List[Dict[str, Any] tc["function"] = {"name": _EMPTY_NAME_SENTINEL, "arguments": "{}"} # --- Drop tool results with a missing/empty tool_call_id --- - # The positional pairing walk below also catches this shape (an id-less - # result expands to zero variants, matches no declared call, and is - # dropped as a positional orphan), but keep the explicit early filter so - # the distinct failure mode keeps its own log line and the guarantee - # doesn't silently depend on the walk's internals: a result with no - # ``tool_call_id`` at all is a schema violation strict OpenAI-compatible - # providers reject outright. ``repair_message_sequence``'s - # Pass 1 already drops this shape (`if tc_id and tc_id in - # known_tool_ids`) when it runs first on the same list, but any caller - # that reaches this function without going through - # ``repair_message_sequence`` first has no such guard. Drop explicitly - # here so this "final chokepoint" claim (see module docstring) actually - # holds regardless of caller (#78071). + # Kept explicit (not left to the positional walk) for its own log line and so the + # final-chokepoint guarantee holds for callers skipping ``repair_message_sequence`` (#78071). _pre_id_filter_count = len(messages) messages = [ m for m in messages @@ -4427,33 +3214,9 @@ def sanitize_api_messages(messages: List[Dict[str, Any]]) -> List[Dict[str, Any] _pre_id_filter_count - len(messages), ) - # --- Positional tool_call <-> tool_result pairing --- - # Strict OpenAI-compatible providers (DeepSeek v4, Kimi) enforce the - # POSITIONAL invariant: an assistant message carrying tool_calls must - # be IMMEDIATELY followed by tool messages covering every - # tool_call_id. The previous implementation compared global id sets, - # which misses the failure mode where a result exists somewhere in - # the transcript but not in the run right after its call — an - # interrupted turn or a compression window can displace a result - # past a user turn. The id then survives in the global result set, - # so the call looks answered, no stub is injected, and the provider - # rejects the payload with HTTP 400 "An assistant message with - # 'tool_calls' must be followed by tool messages responding to each - # 'tool_call_id' (insufficient tool messages following tool_calls - # message)". Rewritten as a single rolling walk on the per-call - # copy (#94704): - # (a) tool results that do not immediately follow an assistant - # message declaring their id are dropped (positional orphans — - # includes results appearing BEFORE their call, which strict - # providers also reject); - # (b) declared ids not covered by the immediately-following tool - # run get a stub result injected at the end of that run, even - # when a mispositioned result exists elsewhere. - # Matching is variant-aware (``tool_call_id_variants`` / - # ``tool_result_id_variants``): a result keyed on ANY alias spelling - # (``id`` / ``call_id`` / ``response_item_id`` / composite bridge) - # answers the call, preserving the unified alias policy from - # #55626/#63000/#93251. + # --- Positional tool_call <-> tool_result pairing (#94704) --- + # Strict providers (DeepSeek v4, Kimi) require results IMMEDIATELY after their call: + # drop positional orphans, stub unanswered declared ids; matching is alias-aware (#55626/#63000/#93251). paired: List[Dict[str, Any]] = [] declared_calls: Dict[str, tuple] = {} dropped_positional_orphans = 0 @@ -4499,17 +3262,13 @@ def sanitize_api_messages(messages: List[Dict[str, Any]]) -> List[Dict[str, Any] ) if matched is not None: paired.append(msg) - # Consume so a duplicate result reusing the id falls into - # the drop branch (same semantics as the old global - # dedup; strict providers reject duplicate tool_call_id). + # Consume so a duplicate result reusing the id is dropped (strict providers reject duplicates). declared_calls.pop(matched, None) else: dropped_positional_orphans += 1 else: if role == "user": - # A user turn closes the tool-result run; subsequent - # tool messages without a fresh declaring assistant - # turn are orphans. + # A user turn closes the tool-result run; later tool messages are orphans. _flush_unanswered_stubs() paired.append(msg) # The transcript may end right after an unanswered assistant turn. @@ -4528,34 +3287,10 @@ def sanitize_api_messages(messages: List[Dict[str, Any]]) -> List[Dict[str, Any] added_stubs, ) - # 3. Deduplicate tool_call_ids. Strict providers (DeepSeek) reject a - # payload where the same tool_call_id appears more than once with HTTP 400 - # "Duplicate value for 'tool_call_id'" (#58327). Duplicates can arise from - # retries, crash/resume glitches, or a compression window that re-emits a - # tool result. This is the final pre-API chokepoint, so dedup defensively - # here even though repair_message_sequence also consumes matched ids. - # (a) collapse duplicate tool_calls WITHIN an assistant message - # (b) drop tool results that answer no OUTSTANDING tool call - # - # (b) tracks outstanding calls rather than every id ever seen, because - # ``tool_call_id`` is NOT globally unique in practice: llama.cpp emits a - # single constant id for every tool call it ever returns (verified: three - # separate completions from one server all carry the same id). A - # seen-once-drop-forever rule reads the SECOND legitimate tool result of - # such a session as a duplicate and deletes it, so from the second tool - # call onward the model never sees any result — it announces its next - # action and the turn dies with the work unfinished. Outstanding-call - # semantics keep both protections intact: a re-emitted result still - # answers no pending call and is still dropped, while a genuine new call - # that reuses the id re-arms that id first. - # Variant-group tracking: answering or deduping one spelling consumes - # its siblings too. A Codex/Responses tool_call registers ``id`` - # (fc_...), ``call_id`` (call_...), ``response_item_id``, and composite - # spellings (#55626/#58168/#63000); tracking only the coalesced id here - # made a result keyed on any OTHER variant look like it answered no - # outstanding call, so this pass deleted the very result step 2's - # variant-aware matching had just preserved (issue #93251 — whole - # parallel batches vanished). + # 3. Deduplicate tool_call_ids (strict providers 400 on duplicates, #58327): collapse + # duplicates within an assistant message; drop results answering no OUTSTANDING call. + # Track outstanding calls (not ids ever seen) because llama.cpp reuses one constant id, + # and track the whole variant group so alias-keyed results are not deleted (#93251). seen_assistant_call_ids: set = set() outstanding_call_ids: set = set() outstanding_groups: Dict[int, frozenset] = {} @@ -4598,10 +3333,7 @@ def sanitize_api_messages(messages: List[Dict[str, Any]]) -> List[Dict[str, Any] removed_dupes += 1 continue if candidate_groups: - # Answered: consume EVERY variant of the matched call so a - # second result replaying any sibling spelling is still - # caught above, and the ids are re-armable by the next - # assistant call that reuses them. + # Consume EVERY variant of the matched call; ids are re-armed by the next call reusing them. group_id = min(candidate_groups) group_variants = outstanding_groups.pop(group_id, frozenset()) for variant in group_variants: @@ -4619,39 +3351,14 @@ def sanitize_api_messages(messages: List[Dict[str, Any]]) -> List[Dict[str, Any] removed_dupes, ) - # 4. Align each tool result's wire-visible ``name`` with the function name - # of the call it answers. Google matches functionResponse.name against - # functionCall.name and rejects a mismatch with HTTP 400 "Request contains - # an invalid argument" (INVALID_ARGUMENT); behind an OpenAI-compatible - # gateway that surfaces only as a generic "Provider returned error". - # - # The mismatch is routine, not corruption. When tool_search defers - # MCP/plugin tools the model calls the bridge tool ``tool_call``, while - # ``make_tool_result_message()`` labels the result with the unwrapped - # internal tool name (``mcp__github__create_issue``) that dispatch, hooks, - # logging, and guardrails need. #72089 fixed exactly this for the native - # Gemini adapter, which now prefers ``tool_name_by_call_id`` over the - # result name; requests that reach Gemini through the OpenAI-compatible - # path (OpenRouter, Vertex/LiteLLM proxies, any OpenAI-shaped gateway) skip - # that translation entirely and still send the internal name on the wire. - # - # Normalizing here rather than in the OpenAI-compat serializer keeps it - # provider-agnostic: Gemini reaches Hermes under many model strings and - # base URLs, so sniffing for "is this really Google?" is unreliable, and - # every other provider either ignores the field or agrees with the call - # name. Runs on the per-call copy, so the stored trajectory keeps the real - # tool name for the session DB and the UI — only the wire payload changes. - # A no-op for the native Gemini path, which already resolves the same name. - # A result whose assistant call frame is missing entirely never reaches - # here — pass 1 above drops it as an orphan — so the only results this pass - # sees are ones whose call name is knowable. + # 4. Align each tool result's wire ``name`` with its call's function name: Google 400s + # on a mismatch, which is routine when tool_search bridges via ``tool_call`` (#72089). + # Done here, provider-agnostically, on the per-call copy only. call_names: Dict[str, str] = {} for msg in messages: if msg.get("role") == "assistant": for tc in msg.get("tool_calls") or []: - # Strip on insert to match the lookup below (and pass 1's - # ``result_call_ids``), so an id that arrives padded still - # pairs instead of silently skipping realignment. + # Strip on insert to match the lookup below so padded ids still pair. cid = (_ra().AIAgent._get_tool_call_id_static(tc) or "").strip() nm = _ra().AIAgent._get_tool_call_name_static(tc) if cid and nm: @@ -4663,10 +3370,7 @@ def sanitize_api_messages(messages: List[Dict[str, Any]]) -> List[Dict[str, Any] cid = (msg.get("tool_call_id") or "").strip() expected = call_names.get(cid) current = msg.get("name") - # Only rewrite a name that is present and disagrees. A result with - # no ``name`` is already valid for Gemini (the id pairs it), so - # leave it absent rather than inventing a field: clean transcripts - # must still pass through byte-identical for prompt caching. + # Only rewrite a present, disagreeing name; clean transcripts must stay byte-identical for prompt caching. if expected and current and current != expected: msg = {**msg, "name": expected} realigned.append((current, expected)) @@ -4692,16 +3396,9 @@ def looks_like_codex_intermediate_ack( ) -> bool: """Detect a planning/ack message that should continue instead of ending the turn. - ``require_workspace`` (default True) keeps the original codex-coding scope: - the ack must reference a filesystem/repo workspace. The conversation loop - passes ``require_workspace=False`` when the user has explicitly opted into - intent-ack continuation for all api_modes (``agent.intent_ack_continuation`` - is ``true`` or a model-list), so general autonomous workflows ("I'll run a - health check on the server", "I'll start the deployment") — which carry a - future-ack and an action verb but no filesystem reference — are caught too. - The future-ack + short-content + no-prior-tools + action-verb requirements - always apply, which is what keeps conversational "I'll help you brainstorm" - replies from tripping it. + ``require_workspace=False`` (user opted into ``agent.intent_ack_continuation`` + for all api_modes) drops the filesystem/repo reference requirement; the + future-ack + short-content + no-prior-tools + action-verb checks always apply. """ if any(isinstance(msg, dict) and msg.get("role") == "tool" for msg in messages): return False @@ -4759,17 +3456,12 @@ def looks_like_codex_intermediate_ack( if not assistant_mentions_action: return False - # Opted-in (all-api_mode) path: a future-ack + action verb + no prior tool - # call is enough — the user asked us to keep going when the model only - # announces intent, regardless of whether a filesystem is involved. + # Opted-in (all-api_mode) path: future-ack + action verb + no prior tool call suffices. if not require_workspace: return True - # ``user_message`` is typed ``str`` but can arrive as an OpenAI-style - # multi-part content list (``[{type:"text",...}, {type:"image_url",...}]``) - # for vision requests routed through the OpenAI-compat API server. A - # truthy list survives ``(user_message or "")`` and then ``.strip()`` - # raises ``AttributeError`` — flatten to text first. + # ``user_message`` may be a multi-part content list (vision via the OpenAI-compat + # server); a list survives ``or ""`` and ``.strip()`` raises, so flatten first. from agent.codex_responses_adapter import _summarize_user_message_for_log user_text = _summarize_user_message_for_log(user_message).strip().lower() @@ -4784,12 +3476,8 @@ def looks_like_codex_intermediate_ack( return user_targets_workspace or assistant_targets_workspace -# Conservative "trailing continue-intent" detector for the said-continue-but- -# stopped stall guard (agent.stall_guards). Matches only when the message TAIL -# announces an immediate next action ("Let me now…", "I will now…", -# "Next, I…"), which is the observed stall shape: the model narrates the next -# step and then ends the turn with no tool call. Kept deliberately narrow so -# ordinary answers that merely contain "I will" mid-sentence never trip it. +# Narrow "trailing continue-intent" detector for the stall guard (agent.stall_guards): +# only the message TAIL announcing a next action, so mid-sentence "I will" never trips it. _TRAILING_CONTINUE_INTENT_RE = re.compile( r"(?:\blet me now\b|\bi(?:['\u2019])?ll now\b|\bi will now\b" r"|\bnow i(?:['\u2019]ll| will)\b|\bnext[,:] i\b)" @@ -4802,13 +3490,7 @@ _TRAILING_CONTINUE_INTENT_MAX_CHARS = 400 def trailing_continue_intent(text: str) -> bool: - """Whether ``text`` is a short reply ENDING on an announced next action. - - Used by the stall-guard extension of the intent-ack continuation path in - ``agent.conversation_loop``: when a turn is about to end with this shape - (no tool calls, short content, trailing intent), the loop re-prompts via - the existing bounded continuation mechanism instead of stopping. - """ + """Whether ``text`` is a short reply ENDING on an announced next action (stall-guard re-prompt trigger).""" t = (text or "").strip() if not t or len(t) > _TRAILING_CONTINUE_INTENT_MAX_CHARS: return False @@ -4816,20 +3498,11 @@ def trailing_continue_intent(text: str) -> bool: def intent_ack_continuation_mode(agent) -> str: - """Classify the resolved intent-ack continuation mode for this turn. + """Resolve the intent-ack continuation mode: ``"off"``, ``"codex_only"`` (workspace acks on codex_responses), or ``"all"``. - Returns one of: - * ``"off"`` — never continue. - * ``"codex_only"`` — historical scope: continue only on the - ``codex_responses`` api_mode, and only for codebase/workspace acks - (``require_workspace=True``). - * ``"all"`` — user opted in for every api_mode; continue on any - future-ack + action verb (``require_workspace=False``). - - Mirrors the four-mode shape of ``agent.tool_use_enforcement``: ``"auto"`` - (default) → codex_only; ``True``/"true"/"always"/"yes"/"on" → all; - ``False``/"false"/"never"/"no"/"off" → off; ``list`` → all when a substring - matches the active model name, else off. + Mirrors ``agent.tool_use_enforcement``: ``"auto"`` -> codex_only; true-ish + values -> all; false-ish -> off; ``list`` -> all when a substring matches + the active model name, else off. """ mode = getattr(agent, "_intent_ack_continuation", "auto") @@ -4844,27 +3517,8 @@ def intent_ack_continuation_mode(agent) -> str: return "codex_only" if agent.api_mode == "codex_responses" else "off" -def intent_ack_continuation_enabled(agent) -> bool: - """Whether intent-ack continuation should fire at all for this turn. - - The ``codex_ack_continuations < 2`` per-turn cap and the - ``looks_like_codex_intermediate_ack`` detector are applied by the caller; - this only decides the on/off gate. Callers that also need to know whether - the workspace requirement applies should use ``intent_ack_continuation_mode`` - directly (``"codex_only"`` ⇒ require_workspace=True, ``"all"`` ⇒ False). - """ - return intent_ack_continuation_mode(agent) != "off" - - - - def copy_reasoning_content_for_api(agent, source_msg: dict, api_msg: dict) -> None: - """Copy provider-facing reasoning fields onto an API replay message. - - Forwarder — the strip-vs-repad POLICY is owned by - ``agent.message_sanitization.apply_reasoning_content_policy`` (audit F4); - this only supplies the agent's cached provider-direction flag. - """ + """Forward reasoning fields onto an API replay message; policy lives in ``agent.message_sanitization.apply_reasoning_content_policy``.""" from agent.message_sanitization import apply_reasoning_content_policy apply_reasoning_content_policy( @@ -4873,33 +3527,12 @@ def copy_reasoning_content_for_api(agent, source_msg: dict, api_msg: dict) -> No def reapply_reasoning_echo_for_provider(agent, api_messages: list) -> int: - """Re-pad (or strip) assistant turns' reasoning_content for the active provider. + """Re-pad or strip assistant turns' reasoning_content for the CURRENT provider after a fallback switch. - ``api_messages`` is built once, before the retry loop, while the *primary* - provider is active. A mid-conversation fallback can then switch providers, - so the reasoning fields baked into ``api_messages`` are shaped for the - *prior* provider and must be reconciled against the *current* one: - - * Switching TO a require-side provider (DeepSeek / Kimi / MiMo thinking - mode): assistant turns built when the prior provider did NOT need the - echo-back go out without ``reasoning_content`` and the new provider - rejects them with HTTP 400 ("The reasoning_content in the thinking mode - must be passed back"). Re-apply the pad. - - * Switching TO a strict provider that rejects the field (Mistral, - Cerebras, Groq, SambaNova, …): assistant turns built under a reasoning - primary carry a ``reasoning_content`` pad (often a single space ``" "``), - and the strict provider rejects it with HTTP 400/422 ("Extra inputs are - not permitted"). Strip the field. This is the exact cross-provider - fallback bug from #45655 — a DeepSeek primary pads history with ``" "``, - the request falls back to Mistral, and Mistral 422s on the stale pad. - - Calling this immediately before building the request kwargs reconciles the - fields against the *current* provider. It is idempotent and safe to call - every iteration; it covers every fallback path. - - Returns the number of assistant turns whose reasoning_content was added or - removed. + ``api_messages`` is shaped for the primary provider; require-side providers + (DeepSeek/Kimi/MiMo) 400 without the pad, strict ones (Mistral, Cerebras, + Groq, ...) 400/422 with it (#45655). Idempotent. Returns the number of + assistant turns changed. """ from agent.message_sanitization import reapply_reasoning_echo @@ -4909,15 +3542,10 @@ def reapply_reasoning_echo_for_provider(agent, api_messages: list) -> int: def _iter_httpx_pool_objects(http_client: Any): - """Yield httpcore pool objects reachable from an httpx client. + """Yield httpcore pool objects reachable from an httpx client, including mounted transports. - Hermes' keepalive client (#10324 / ``_build_keepalive_http_client``) and - any ``HTTP(S)_PROXY`` configuration put live connections on *mounted* - transports (``client._mounts``), not only on the default - ``client._transport``. Walking the default transport alone makes - ``force_close_tcp_sockets`` return 0 while a stream is still mid-recv — - the interrupt logs success and the provider keeps burning the slot - (#72975). + Keepalive (#10324) and proxy configs put live connections on ``client._mounts``; + walking only ``_transport`` made ``force_close_tcp_sockets`` miss them (#72975). """ seen_pools: set[int] = set() @@ -4933,10 +3561,8 @@ def _iter_httpx_pool_objects(http_client: Any): def _pools_for_transport(transport: Any): if transport is None: return - # Normal httpx.HTTPTransport / HTTPProxy-as-transport: connections - # live under ``_pool``. HTTPProxy itself *is* a ConnectionPool and - # may be mounted directly — then ``_connections`` is on the - # transport. + # Connections live under ``_pool``; a directly mounted HTTPProxy *is* a + # ConnectionPool, so ``_connections`` may sit on the transport itself. pool = getattr(transport, "_pool", None) if pool is not None: yield from _emit(pool) @@ -4974,22 +3600,15 @@ def _connection_candidates(conn: Any): def _iter_pool_sockets(client: Any): """Yield raw sockets reachable from an OpenAI/httpx client pool. - httpcore 1.x stores the concrete HTTP11/HTTP2 connection under - ``conn._connection``; older versions exposed stream attributes directly - on the pool entry. Proxy tunnels wrap another layer - (``TunnelHTTPConnection`` / ``ForwardHTTPConnection``). Keep the - traversal defensive because these are private transport internals and - vary across httpx/httpcore releases. - - Also walks ``httpx`` mount transports — see ``_iter_httpx_pool_objects`` - — and in-flight httpcore ``PoolRequest.connection`` objects, which stay - reachable even when ``_connections`` is empty during checkout (#85252). + Traversal is defensive over private httpcore internals (``conn._connection``, + proxy tunnel wrappers) that vary by release. Also walks mount transports and + in-flight ``PoolRequest.connection`` objects, reachable when + ``_connections`` is empty during checkout (#85252). """ try: http_client = getattr(client, "_client", None) if http_client is None: - # Some SDK wrappers *are* the httpx client (or expose the pool - # directly). Fall through so mount-aware discovery still runs. + # Some SDK wrappers *are* the httpx client; fall through so mount-aware discovery runs. http_client = client pools = list(_iter_httpx_pool_objects(http_client)) except Exception: @@ -5000,9 +3619,7 @@ def _iter_pool_sockets(client: Any): seen: set[int] = set() for pool in pools: - # Empty-list is falsy: use ``is None`` so an empty ``_connections`` - # still lets us walk in-flight ``_requests`` rather than skipping - # the pool entirely. + # ``is None``, not falsiness: an empty ``_connections`` must still let us walk in-flight ``_requests``. raw_conns = getattr(pool, "_connections", None) if raw_conns is None: raw_conns = getattr(pool, "_pool", None) @@ -5052,14 +3669,7 @@ def _iter_pool_sockets(client: Any): def cleanup_dead_connections(agent) -> bool: - """Detect and clean up dead TCP connections on the primary client. - - Inspects the httpx connection pool for sockets in unhealthy states - (CLOSE-WAIT, errors). If any are found, force-closes all sockets - and rebuilds the primary client from scratch. - - Returns True if dead connections were found and cleaned up. - """ + """Force-close and rebuild the primary client if its pool has dead sockets (CLOSE-WAIT, errors); returns True if cleaned.""" client = getattr(agent, "client", None) if client is None: return False @@ -5180,27 +3790,17 @@ def extract_api_error_context(error: Exception) -> Dict[str, Any]: def apply_pending_steer_to_tool_results(agent, messages: list, num_tool_msgs: int) -> None: - """Append any pending /steer text to the last tool result in this turn. + """Append pending /steer text to the last ``role:"tool"`` message of this batch, marked as user-origin. - Called at the end of a tool-call batch, before the next API call. - The steer is appended to the last ``role:"tool"`` message's content - with a clear marker so the model understands it came from the user - and NOT from the tool itself. Role alternation is preserved — - nothing new is inserted, we only modify existing content. - - Args: - messages: The running messages list. - num_tool_msgs: Number of tool results appended in this batch; - used to locate the tail slice safely. + Modifies existing content only, so role alternation is preserved. + ``num_tool_msgs`` bounds the tail slice searched. """ if num_tool_msgs <= 0 or not messages: return steer_text = agent._drain_pending_steer() if not steer_text: return - # Find the last tool-role message in the recent tail. Skipping - # non-tool messages defends against future code appending - # something else at the boundary. + # Skip non-tool messages in the tail in case something else is appended at the boundary. target_idx = None for j in range(len(messages) - 1, max(len(messages) - num_tool_msgs - 1, -1), -1): msg = messages[j] @@ -5208,9 +3808,8 @@ def apply_pending_steer_to_tool_results(agent, messages: list, num_tool_msgs: in target_idx = j break if target_idx is None: - # No tool result in this batch (e.g. all skipped by interrupt); - # put the steer back so the caller's fallback path can deliver - # it as a normal next-turn user message. + # No tool result in this batch (e.g. all skipped by interrupt): put the steer + # back so the caller's fallback delivers it as a next-turn user message. _lock = getattr(agent, "_pending_steer_lock", None) if _lock is not None: with _lock: @@ -5225,8 +3824,7 @@ def apply_pending_steer_to_tool_results(agent, messages: list, num_tool_msgs: in marker = format_steer_marker(steer_text) existing_content = messages[target_idx].get("content", "") if not isinstance(existing_content, str): - # Anthropic multimodal content blocks — preserve them and append - # a text block at the end. + # Anthropic multimodal content blocks: preserve them and append a text block. try: blocks = list(existing_content) if existing_content else [] blocks.append({"type": "text", "text": marker.lstrip()}) @@ -5245,40 +3843,15 @@ def apply_pending_steer_to_tool_results(agent, messages: list, num_tool_msgs: in def force_close_tcp_sockets(client: Any) -> int: - """Abort in-flight TCP I/O by shutting down sockets WITHOUT closing FDs. + """Abort in-flight TCP I/O via ``shutdown(SHUT_RDWR)`` WITHOUT closing FDs. - When a provider drops a connection mid-stream — or the user issues an - interrupt — we want to unblock httpx's reader/writer immediately rather - than waiting for the kernel's per-connection timeout. ``shutdown(SHUT_RDWR)`` - achieves that: it sends FIN, breaks any pending ``recv``/``send`` with EOF - or ``EPIPE``, but does NOT release the file descriptor. + ``close()`` from a non-owner thread is unsafe: the SSL BIO caches the raw + FD, the kernel recycles it, and a flushed TLS record lands in the wrong + file (#29507 clobbered a SQLite header). ``shutdown()`` is FD-safe from + any thread; the owning httpx thread releases the FD on unwind. - Historically this helper also called ``socket.close()`` so the FD got - released immediately, but that's unsafe when (as is the case for both the - interrupt-abort path and stale-call kill path) the helper runs on a - different thread than the one driving the request: - - * The Python ``socket.socket`` we close here is the SAME object held by - httpx's pool, so closing it via Python sets its ``_fd`` to -1 and - future operations on that Python object fail safely. - * BUT the SSL wrapper (``ssl.SSLSocket``'s underlying OpenSSL ``BIO``) - caches the raw integer FD. Once ``os.close(fd)`` runs, the kernel may - immediately recycle that integer to the next ``open()`` call — e.g. - the kanban dispatcher opening ``kanban.db``. - * The owning worker thread then unwinds httpx, the SSL layer flushes a - pending TLS record, and the encrypted bytes get written into the - wrong file (issue #29507: 24-byte TLS application-data record - clobbering SQLite header bytes 5..28). - - The fix is to let the owning thread own the close. ``shutdown()`` from any - thread is FD-safe; ``close()`` is not. The httpx connection's own close - path — which runs from the worker thread when it unwinds — will release - the FD via the same ``socket.socket`` object, and because Python's socket - close atomically swaps ``_fd`` to -1 *before* issuing ``os.close``, there - is no FD-aliasing window when only one thread closes. - - Returns the number of sockets shut down. (Field kept as - ``tcp_force_closed=N`` in the log line for backwards-compatible parsing.) + Returns the number of sockets shut down (logged as ``tcp_force_closed=N`` + for backwards-compatible parsing). """ import socket as _socket @@ -5286,10 +3859,8 @@ def force_close_tcp_sockets(client: Any) -> int: try: for sock in _iter_pool_sockets(client): try: - # Clear a blocking timeout first so a hung SSL_read on the - # owner thread notices the shutdown. Some stacks ignore - # SHUT_RDWR alone while recv is blocked with timeout=None - # (#85252). Still no close() — that is the #29507 race. + # Clear a blocking timeout so a hung SSL_read notices the shutdown (#85252). + # Still no close() — that is the #29507 race. settimeout = getattr(sock, "settimeout", None) if callable(settimeout): try: diff --git a/agent/inline_tool_executors.py b/agent/inline_tool_executors.py new file mode 100644 index 0000000000..da5c8b6c81 --- /dev/null +++ b/agent/inline_tool_executors.py @@ -0,0 +1,292 @@ +"""Agent-level ("inline") tool executors shared by the sequential and concurrent tool paths. + +These tools need live ``AIAgent`` state (stores, callbacks, session DB) and therefore +bypass the tool registry. Each executor is ``fn(agent, args, ctx) -> result``; the +table replaces two hand-maintained if/elif chains (``invoke_tool`` and +``execute_tool_calls_sequential``) that had drifted apart. Tool modules are imported +lazily inside the bodies so ``patch("tools.x.y")`` in tests keeps working. +""" + +from __future__ import annotations + +import json +from dataclasses import dataclass +from typing import Any, Callable, Dict, Optional + + +def tool_hook_ids(agent, effective_task_id: str, tool_call_id: Optional[str]) -> Dict[str, str]: + """Identity kwargs every tool hook/middleware call carries (all coerced to ``""``).""" + return { + "task_id": effective_task_id or "", + "session_id": getattr(agent, "session_id", "") or "", + "tool_call_id": tool_call_id or "", + "turn_id": getattr(agent, "_current_turn_id", "") or "", + "api_request_id": getattr(agent, "_current_api_request_id", "") or "", + } + + +def emit_terminal_post_tool_call( + agent, + *, + function_name: str, + function_args: dict, + result: Any, + effective_task_id: str, + tool_call_id: Optional[str], + duration_ms: int = 0, + status: Optional[str] = None, + error_type: Optional[str] = None, + error_message: Optional[str] = None, + middleware_trace: Optional[list] = None, +) -> None: + """Emit the one terminal ``post_tool_call`` hook for a tool_call_id (best-effort).""" + try: + from model_tools import _emit_post_tool_call_hook + _emit_post_tool_call_hook( + function_name=function_name, + function_args=function_args, + result=result, + **tool_hook_ids(agent, effective_task_id, tool_call_id), + duration_ms=duration_ms, + status=status, + error_type=error_type, + error_message=error_message, + middleware_trace=list(middleware_trace or []), + ) + except Exception: + pass + + +@dataclass +class InlineToolContext: + """Per-call state an inline executor may need beyond its arguments.""" + + effective_task_id: str + tool_call_id: Optional[str] = None + messages: Optional[list] = None + + +def _todo_list(agent, args: dict, ctx: InlineToolContext) -> Any: + from tools.todo_tool import todo_tool as _todo_tool + + return _todo_tool( + todos=args.get("todos"), + merge=args.get("merge", False), + store=agent._todo_store, + ) + + +def _message_agent(agent, args: dict, ctx: InlineToolContext) -> Any: + # Bot Mode teammate DM is injected, not registered: only a canonical Bot + # Chat session carries the schema, and the tool re-gates on the title. + from tools.bot_mode_dm import message_agent_tool as _message_agent_tool + + return _message_agent_tool( + target=args.get("target", ""), + message=args.get("message", ""), + task_id=ctx.effective_task_id, + agent=agent, + ) + + +def _session_search(agent, args: dict, ctx: InlineToolContext) -> Any: + session_db = agent._get_session_db_for_recall() + if not session_db: + from hermes_state import format_session_db_unavailable + + return json.dumps({"success": False, "error": format_session_db_unavailable()}) + from tools.session_search_tool import session_search as _session_search_tool + + return _session_search_tool( + query=args.get("query", ""), + role_filter=args.get("role_filter"), + limit=args.get("limit", 3), + session_id=args.get("session_id"), + around_message_id=args.get("around_message_id"), + window=args.get("window", 5), + sort=args.get("sort"), + detail=args.get("detail", "adaptive"), + db=session_db, + current_session_id=agent.session_id, + ) + + +def _memory(agent, args: dict, ctx: InlineToolContext) -> Any: + from tools.memory_tool import memory_tool as _memory_tool + + result = _memory_tool( + action=args.get("action"), + target=args.get("target", "memory"), + content=args.get("content"), + old_text=args.get("old_text"), + operations=args.get("operations"), + store=agent._memory_store, + ) + # Mirror built-in memory writes to external providers; gating lives in + # MemoryManager.notify_memory_tool_write. + if agent._memory_manager: + agent._memory_manager.notify_memory_tool_write( + result, + args, + build_metadata=lambda: agent._build_memory_write_metadata( + task_id=ctx.effective_task_id, + tool_call_id=ctx.tool_call_id, + ), + ) + return result + + +def _clarify(agent, args: dict, ctx: InlineToolContext) -> Any: + from tools.clarify_tool import clarify_tool as _clarify_tool + + return _clarify_tool( + question=args.get("question", ""), + choices=args.get("choices"), + multi_select=args.get("multi_select", False), + questions=args.get("questions"), + callback=agent.clarify_callback, + ) + + +def _read_terminal(agent, args: dict, ctx: InlineToolContext) -> Any: + from tools.read_terminal_tool import read_terminal_tool as _read_terminal_tool + + return _read_terminal_tool( + start_line=args.get("start_line"), + count=args.get("count"), + callback=getattr(agent, "read_terminal_callback", None), + ) + + +def _desktop_preview(agent, args: dict, ctx: InlineToolContext) -> Any: + # action=read needs the GUI callback (agent-level); open/close go through the + # registry handler like any other tool. + if (args.get("action") or "").strip() == "read": + from tools.read_preview_tool import read_preview_tool as _read_preview_tool + + return _read_preview_tool( + start=args.get("start"), + count=args.get("count"), + callback=getattr(agent, "read_preview_callback", None), + ) + from tools.preview_tool import _handle_preview + + return _handle_preview(args) + + +def _drive_preview(agent, args: dict, ctx: InlineToolContext) -> Any: + from tools.drive_preview_tool import drive_preview_tool as _drive_preview_tool + + return _drive_preview_tool( + action=args.get("action", ""), + ref=args.get("ref"), + selector=args.get("selector"), + text=args.get("text"), + key=args.get("key"), + submit=args.get("submit"), + amount=args.get("amount"), + to=args.get("to"), + limit=args.get("max"), + callback=getattr(agent, "drive_preview_callback", None), + ) + + +def _annotate_preview(agent, args: dict, ctx: InlineToolContext) -> Any: + from tools.annotate_preview_tool import annotate_preview_tool as _annotate_preview_tool + + return _annotate_preview_tool( + action=args.get("action", "add"), + ref=args.get("ref"), + selector=args.get("selector"), + label=args.get("label"), + callback=getattr(agent, "drive_preview_callback", None), + ) + + +def _read_window_below(agent, args: dict, ctx: InlineToolContext) -> Any: + from tools.read_window_tool import read_window_below_tool as _read_window_below_tool + + return _read_window_below_tool( + callback=getattr(agent, "read_window_below_callback", None), + ) + + +def _gui_tour(agent, args: dict, ctx: InlineToolContext) -> Any: + from tools.tour_tool import tour_tool as _tour_tool + + return _tour_tool( + action=args.get("action", ""), + surface=args.get("surface"), + selector=args.get("selector"), + title=args.get("title"), + text=args.get("text"), + side=args.get("side"), + steps=args.get("steps"), + step_index=args.get("step_index"), + callback=getattr(agent, "tour_callback", None), + ) + + +def _setup_mcp(agent, args: dict, ctx: InlineToolContext) -> Any: + from tools.setup_mcp_tool import setup_mcp_tool as _setup_mcp_tool + + return _setup_mcp_tool( + server=args.get("server", ""), + action=args.get("action", "install"), + reason=args.get("reason", ""), + callback=getattr(agent, "setup_mcp_callback", None), + ) + + +def _delegate_task(agent, args: dict, ctx: InlineToolContext) -> Any: + return agent._dispatch_delegate_task(args) + + +InlineToolExecutor = Callable[[Any, dict, InlineToolContext], Any] + +# Order is the historical if/elif order of ``execute_tool_calls_sequential``. +INLINE_TOOL_EXECUTORS: Dict[str, InlineToolExecutor] = { + "todo_list": _todo_list, + "message_agent": _message_agent, + "session_search": _session_search, + "memory": _memory, + "clarify": _clarify, + "read_terminal": _read_terminal, + "desktop_preview": _desktop_preview, + "drive_preview": _drive_preview, + "annotate_preview": _annotate_preview, + "read_window_below": _read_window_below, + "gui_tour": _gui_tour, + "setup_mcp": _setup_mcp, + "delegate_task": _delegate_task, +} + +# ``invoke_tool`` (concurrent path) historically consulted the memory manager right +# after these three names and before the remaining inline tools; it never handled +# ``message_agent`` inline (that name falls through to the registry there). +INVOKE_TOOL_PRE_MEMORY_MANAGER_NAMES = frozenset({"todo_list", "session_search", "memory"}) + + +def memory_manager_executor(function_name: str) -> InlineToolExecutor: + """Executor routing ``function_name`` through ``agent._memory_manager``.""" + + def _run(agent, args: dict, ctx: InlineToolContext) -> Any: + return agent._memory_manager.handle_tool_call(function_name, args) + + return _run + + +def resolve_invoke_tool_executor(agent, function_name: str) -> Optional[InlineToolExecutor]: + """Inline executor for ``invoke_tool`` (concurrent path), or None for registry dispatch. + + Preserves the historical precedence: todo_list/session_search/memory, then memory + manager tools, then the remaining inline tools (``message_agent`` excluded). + """ + if function_name in INVOKE_TOOL_PRE_MEMORY_MANAGER_NAMES: + return INLINE_TOOL_EXECUTORS[function_name] + memory_manager = agent._memory_manager + if memory_manager and memory_manager.has_tool(function_name): + return memory_manager_executor(function_name) + if function_name == "message_agent": + return None + return INLINE_TOOL_EXECUTORS.get(function_name) diff --git a/agent/tool_executor.py b/agent/tool_executor.py index f1a04c3718..af1b3c676b 100644 --- a/agent/tool_executor.py +++ b/agent/tool_executor.py @@ -1,13 +1,7 @@ -"""Tool-call execution — sequential and concurrent dispatch. +"""Tool-call execution: sequential and concurrent dispatch, extracted from AIAgent. -Both AIAgent methods (``_execute_tool_calls_sequential`` and -``_execute_tool_calls_concurrent``) live here as module-level -functions that take the parent ``AIAgent`` as their first argument. - -``run_agent`` keeps thin wrappers so existing call sites work; tests -that patch ``run_agent._set_interrupt`` are honored because the -extracted functions reach back through the ``run_agent`` module via -``_ra()`` for that symbol. +Functions take the parent ``AIAgent`` first; ``run_agent`` keeps thin wrappers, and +tests that patch ``run_agent._set_interrupt`` still work because we reach it via ``_ra()``. """ from __future__ import annotations @@ -33,6 +27,12 @@ from agent.display import ( _detect_tool_failure, ) from agent.message_sanitization import coalesce_tool_call_id +from agent.inline_tool_executors import ( + INLINE_TOOL_EXECUTORS, + InlineToolContext, + emit_terminal_post_tool_call, + tool_hook_ids, +) from agent.tool_dispatch_helpers import ( _NEVER_PARALLEL_TOOLS, _is_destructive_command, @@ -62,12 +62,9 @@ def _pairing_tool_call_id(tool_call: Any) -> str: def _record_persisted_path_for_stub(agent, tool_call_id: str, function_result) -> None: - """Tell the stall guards where a persisted result's full content lives. + """Record the spillover file path so a later result-reference stub can't dangle. - When a large result is spilled to disk ( preview), a - later result-reference stub pointing at that first occurrence must carry - the spillover file path so the reference can't dangle. Best-effort: never - lets bookkeeping break tool execution. + Best-effort: bookkeeping never breaks tool execution. """ try: if not isinstance(function_result, str): @@ -90,10 +87,8 @@ def _ensure_file_checkpoint( if not file_path: return - # File tools resolve relative paths against the task's live/session cwd, - # which can differ from the Hermes process cwd (notably in Docker). Resolve - # through that same path pipeline before asking the checkpoint manager to - # discover the project root. + # File tools resolve relative paths against the task's live cwd (differs from the + # process cwd in Docker); resolve the same way before locating the project root. from tools.file_tools import _resolve_path_for_task resolved_path = _resolve_path_for_task(file_path, effective_task_id or "default") @@ -104,17 +99,13 @@ def _ensure_file_checkpoint( def _budget_for_agent(agent) -> BudgetConfig: """Resolve a tool-result BudgetConfig scaled to the agent's context window. - Large-context models keep the historical 100K/200K char defaults; small - models (e.g. a 65K-token local model switched into mid-session) get a budget - proportional to their window so a single large tool result can't push the - request past the model's limit (#23767). Falls back to the default budget - when the context length isn't resolvable. + Small-context models get a proportional budget so one large result can't overflow + the request (#23767); falls back to the default when context length is unknown. """ try: ctx = getattr(getattr(agent, "context_compressor", None), "context_length", None) - # budget_for_context_window(None) (rather than DEFAULT_BUDGET) so the - # config-driven MCP threshold override still applies when the context - # length isn't resolvable. + # budget_for_context_window(None), not DEFAULT_BUDGET, so the MCP threshold + # override still applies when the context length isn't resolvable. return budget_for_context_window(int(ctx) if ctx else None) except Exception: return DEFAULT_BUDGET @@ -126,40 +117,26 @@ _DEFAULT_IMAGE_PARALLEL_REQUESTS = 4 # Generous ceiling for slow-but-valid tool work (large page fetches, slow # remote backends) so the batch guard does not preempt a legitimate attempt. _DEFAULT_CONCURRENT_TOOL_TIMEOUT_S = 420.0 -# Upper bound a concurrent worker will wait at the start-order gate for all -# earlier-ordered tools to advance before proceeding out of order. Long enough -# to cover slow-but-legitimate authorization (e.g. an approval round-trip), -# short enough that one wedged dispatch cannot starve the batch forever. +# Start-order gate wait bound: long enough for an approval round-trip, short enough +# that one wedged dispatch cannot starve the batch. _START_ORDER_GATE_TIMEOUT_S = 120.0 -# Fallback bound a concurrent worker will wait for the authorization gate's -# serialization lock before running its prompt unserialized. The effective -# bound is derived from ``approvals.timeout`` plus a margin (see -# _authorization_gate_lock_timeout): a legitimate holder is at worst a human -# answering an approval prompt, which self-terminates at approvals.timeout — -# so a holder that overstays it is wedged and must not starve the batch. +# Fallback authorization-gate lock bound; the effective bound derives from +# approvals.timeout (see _authorization_gate_lock_timeout) since overstaying it means wedged. _AUTHORIZATION_GATE_LOCK_TIMEOUT_S = 360.0 def _authorization_gate_lock_timeout() -> float: """Bound for the authorization serialization lock: approval timeout + margin. - Delegates to ``tools.approval.human_wait_ceiling`` — the same bound that - clamps a human-wait window's deadline contribution — so the two can't - drift. Long enough that serialization is never broken while a legitimate - approval prompt is still answerable; short enough that a wedged holder - (hanging ``pre_tool_call`` plugin, dead approval client) cannot park other - workers forever (#79719). Resolved once per gate (per batch), so a - mid-process ``approvals.timeout`` change applies from the next batch. + Delegates to ``tools.approval.human_wait_ceiling`` so the two bounds can't drift: + never break serialization while an approval prompt is answerable, but never let a + wedged holder park other workers forever (#79719). Resolved once per batch. """ try: from tools.approval import human_wait_ceiling - # human_wait_ceiling is platform-safety-capped (agent/deadline.py - # MAX_SAFE_TIMEOUT_S): a huge approvals.timeout can no longer overflow - # Lock.acquire's time_t on macOS (#83220). Deliberately NOT min()'d - # with _AUTHORIZATION_GATE_LOCK_TIMEOUT_S — the gate must never give - # up while a legitimate approval prompt is still answerable (#79719), - # so a configured approvals.timeout above 360s must extend the gate. + # Safety-capped so a huge approvals.timeout can't overflow Lock.acquire (#83220); + # deliberately NOT min()'d with the fallback so the gate never gives up early (#79719). return human_wait_ceiling() except Exception: return _AUTHORIZATION_GATE_LOCK_TIMEOUT_S @@ -168,8 +145,7 @@ def _authorization_gate_lock_timeout() -> float: class _BatchAbandoned(BaseException): """Raised inside a worker when the batch was abandoned before dispatch. - Derives from BaseException so intermediate ``except Exception`` handlers in - the middleware chain cannot swallow it and dispatch the tool anyway. + BaseException so ``except Exception`` handlers in the middleware chain can't swallow it. """ @@ -193,11 +169,10 @@ def _parse_tool_arguments(raw_arguments: Any) -> tuple[dict, Optional[str]]: def _resolve_concurrent_tool_timeout() -> float | None: - """Resolve the per-batch concurrent tool deadline. + """Resolve the per-batch concurrent tool deadline via the unified resolver (#85125). - Delegates to the unified resolver (#85125): ``timeouts.tools.concurrent_batch`` - in config.yaml wins, the legacy ``HERMES_CONCURRENT_TOOL_TIMEOUT_S`` env var - remains the back-compat bridge, and ``0``/negative still disables the bound. + ``timeouts.tools.concurrent_batch`` wins; ``HERMES_CONCURRENT_TOOL_TIMEOUT_S`` is the + legacy bridge; ``0``/negative disables the bound. """ from agent.deadline import resolve_timeout @@ -214,20 +189,16 @@ def _flush_session_db_after_tool_progress( *, stage: str, ) -> bool: - """Flush tool-call progress before projecting it to any UI surface. + """Flush tool-call progress to the session DB before projecting it to any UI. - Tool execution can perform side effects that terminate or restart the - current Hermes process before the normal turn-end persistence path runs. - Flush the already-appended assistant/tool messages immediately so the - transcript survives destructive-but-valid tool calls. + Tool side effects can kill/restart the process before turn-end persistence runs. """ try: persisted = agent._flush_messages_to_session_db(messages) is not False if not persisted: agent._incremental_persistence_failed = True - # The flush caught its own exception and returned False; the - # classified cause (if any) was captured at the catch site. Only - # fall back to 'unknown' when nothing more specific is recorded. + # Flush recorded any classified cause at the catch site; only default + # to 'unknown' when nothing more specific exists. if getattr(agent, "_last_persistence_error_cause", None) is None: agent._last_persistence_error_cause = "unknown" return persisted @@ -240,11 +211,8 @@ def _flush_session_db_after_tool_progress( def _image_generate_parallel_limit() -> int: - """Return the configured image-generation parallelism cap. - - Image-generation calls are slow enough that concurrent execution is useful, - but backend bursts can hit TTFB or rate-limit failures. Keep the default - intentionally conservative while allowing users to tune it per install. + """Return the configured image-generation parallelism cap (conservative default; + backend bursts hit TTFB/rate-limit failures). """ try: from hermes_cli.config import load_config @@ -286,50 +254,15 @@ def _ra(): def _is_interpreter_shutdown_submit_error(exc: RuntimeError) -> bool: - """Shutdown-race predicate — shared home in ``tools.interpreter_shutdown``. - - Delegates so all sites (cron delivery, conversation-loop retry, tool - submission) recognize both CPython shutdown-message variants instead of - each matching its own substring (the bug class behind #55924/#58720). + """Shutdown-race predicate; delegates to ``tools.interpreter_shutdown`` so every site + recognizes both CPython shutdown-message variants (#55924/#58720). """ from tools.interpreter_shutdown import interpreter_shutting_down return interpreter_shutting_down(exc) -def _emit_terminal_post_tool_call( - agent, - *, - function_name: str, - function_args: dict, - result: Any, - effective_task_id: str, - tool_call_id: str, - duration_ms: int = 0, - status: str | None = None, - error_type: str | None = None, - error_message: str | None = None, - middleware_trace: Optional[list[dict[str, Any]]] = None, -) -> None: - try: - from model_tools import _emit_post_tool_call_hook - _emit_post_tool_call_hook( - function_name=function_name, - function_args=function_args, - result=result, - task_id=effective_task_id or "", - session_id=getattr(agent, "session_id", "") or "", - tool_call_id=tool_call_id or "", - turn_id=getattr(agent, "_current_turn_id", "") or "", - api_request_id=getattr(agent, "_current_api_request_id", "") or "", - duration_ms=duration_ms, - status=status, - error_type=error_type, - error_message=error_message, - middleware_trace=list(middleware_trace or []), - ) - except Exception: - pass +_emit_terminal_post_tool_call = emit_terminal_post_tool_call def _cancelled_tool_result(reason: str = "user interrupt") -> str: @@ -374,17 +307,9 @@ def _emit_cancelled_terminal_post_tool_call( def _tool_search_scoped_names(agent) -> frozenset: """Return the deferrable tool names the session may invoke via tool_call. - The Tool Search unwrap dispatches the underlying tool directly, bypassing - the bridge branch (and its scope check) in - ``model_tools.handle_function_call``. To keep a restricted-toolset session - (subagent, kanban worker, curated gateway session) from reaching tools it - was never granted, the unwrap validates the underlying name against this - set: the deferrable subset of the session's own enabled/disabled toolset - scope. - - Result is cached on the agent and refreshed when the tool registry's - generation changes (e.g. an MCP server reconnects), so the common case is - a dict lookup, not a full tool-defs rebuild on every tool call. + The Tool Search unwrap bypasses the bridge's scope check in + ``model_tools.handle_function_call``, so restricted sessions are validated against + this set. Cached on the agent; refreshed when the registry generation changes. """ try: import model_tools @@ -421,6 +346,60 @@ def _tool_search_scoped_names(agent) -> frozenset: return names +def _canonical_tool_name(function_name: str) -> str: + """Map legacy tool-name aliases (2026-08 renames) BEFORE agent-loop dispatch.""" + from model_tools import _LEGACY_TOOL_ALIASES as _lta + + return _lta.get(function_name, function_name) + + +def _unwrap_tool_search_call( + agent, function_name: str, function_args: dict, *, flatten_probe: bool = False +) -> tuple[str, dict, Optional[str]]: + """Peel the ``tool_call`` bridge so downstream hooks see the underlying tool. + + Checkpointing, guardrails, plugin hooks and the activity feed must observe the real + tool name, not the bridge. ``tool_call.function`` stays untouched for the transcript + and tool_call_id pairing. The unwrap bypasses + handle_function_call's scope check, so session toolset scope is enforced HERE. + Returns ``(name, args, scope_block)``; ``scope_block`` is the block message when + the underlying tool is out of scope or its args fail the deferred-schema probe + (``flatten_probe`` collapses the probe's JSON payload to one plain string for + callers that wrap the message in ``{"error": ...}``). + """ + scope_block: Optional[str] = None + try: + from tools import tool_search as _ts + if function_name == _ts.TOOL_CALL_NAME: + underlying, underlying_args, err = _ts.resolve_underlying_call(function_args) + if not err and underlying: + if underlying in _tool_search_scoped_names(agent): + # Validate before unwrapping: the generic bridge hides the concrete + # parameter schema from provider-native tool-call validation. + probe_err = _ts.validate_deferred_call_args(underlying, underlying_args) + if probe_err is None: + return underlying, underlying_args, None + scope_block = probe_err + if flatten_probe: + try: + probe = json.loads(probe_err) + scope_block = ( + f"{probe.get('error', '')} Parameters schema: " + f"{json.dumps(probe.get('parameters', {}), ensure_ascii=False)}. " + f"{probe.get('hint', '')}" + ).strip() + except Exception: + scope_block = probe_err + else: + scope_block = ( + f"'{underlying}' is not available in this session. " + "Use tool_search to find tools you can call." + ) + except Exception: + pass + return function_name, function_args, scope_block + + @dataclass class _ManagedToolResult: result: Any @@ -437,33 +416,20 @@ class _ToolTimeoutResult(str): class _ToolCancelledResult(str): """Marker for a synthesized sequential-tool user-interrupt result. - Like ``_ToolTimeoutResult``, the executor already emitted the terminal - post_tool_call event for this call (status="cancelled"), so downstream - emission must be suppressed — an abandoned worker finishing late must not - report success for a call the user already cancelled. + The terminal post_tool_call event was already emitted (status=cancelled), so a + late-finishing abandoned worker must not report success. """ class _ConcurrentToolAuthorizationGate: """Serialize policy prompts and exclude human approval waits from batch deadlines. - Serialization keeps concurrent approval prompts from interleaving on the - user's screen. The acquire is BOUNDED: a worker wedged inside the gate (a - hanging ``pre_tool_call`` plugin, or an approval round-trip to a client - that went away) must not park every other worker forever. On expiry the - worker runs its prompt unserialized — worst case is interleaved prompts, - strictly better than permanent starvation (same tradeoff as the - start-order gate, #79705). + The acquire is BOUNDED: on expiry the worker prompts unserialized rather than + starving the batch behind a wedged plugin/approval client (#79705). Deadline exclusion is measured at the SOURCE of the human wait - (``tools.approval.human_wait_seconds``: the CLI prompt and the gateway - approval poll loop mark their own blocking windows), NOT as residency in - this gate. Gate residency is arbitrary code — using it as the exclusion - signal let a wedged plugin grow the exclusion 1:1 with wall clock, keeping - the batch deadline's ``remaining`` constant so it never fired and the turn - hung forever (#79719). A wedged plugin now contributes nothing to the - exclusion and the batch times out normally, while a genuine approval wait - (which can legitimately exceed any fixed bound) is still excluded in full. + (``tools.approval.human_wait_seconds``), NOT as gate residency: residency-based + exclusion let a wedged plugin keep the deadline from ever firing (#79719). """ def __init__( @@ -483,9 +449,8 @@ class _ConcurrentToolAuthorizationGate: try: from tools.approval import get_current_session_key - # Snapshot the batch's session identity on the SUBMITTING - # thread: excluded_seconds() is polled from the batch wait - # loop, whose context may differ from the workers'. + # Snapshot on the SUBMITTING thread: excluded_seconds() is polled + # from the batch wait loop, whose context may differ from workers'. self._session_key = get_current_session_key() except Exception: logger.debug( @@ -535,9 +500,8 @@ def _managed_values( ) -# Cadence for the in-flight tool activity heartbeat. Must stay far below the -# gateway turn-inactivity timeout (default 1800s) so a silent-but-healthy -# tool call never looks idle to the watchdog. +# Heartbeat cadence; must stay far below the gateway turn-inactivity timeout +# (default 1800s) so a silent-but-healthy tool never looks idle. _TOOL_ACTIVITY_HEARTBEAT_INTERVAL_S = 30.0 @@ -547,28 +511,11 @@ def _run_tool_activity_heartbeat( label: str, interval: float = _TOOL_ACTIVITY_HEARTBEAT_INTERVAL_S, ) -> None: - """Refresh the agent's activity clock while a tool call is in flight. + """Daemon thread that stamps ``agent._touch_activity`` every ``interval`` seconds + until ``stop_event`` is set. - The gateway's turn-inactivity watchdog - (``gateway/run.py::_watch_gateway_turn_inactivity``) abandons a turn - once ``seconds_since_activity`` exceeds the inactivity timeout - (default 30 min). Activity is stamped when a tool *starts* and when it - *completes*, but a tool call that runs silently for 30+ minutes - (quiet builds, long pytest suites, large downloads, network waits that - emit no output) previously froze the clock at "executing tool: " - and the watchdog hard-abandoned a turn that was still making progress, - reaping the tool's processes mid-execution. - - This daemon thread touches ``agent._touch_activity`` every ``interval`` - seconds until ``stop_event`` is set (the tool call returned), so the - gateway keeps seeing a live turn for the whole duration of the call. - - A tool that truly hangs is still bounded by the tool layer's own - timeouts (terminal ``timeout`` default 180s, the concurrent batch - deadline ~420s), so the heartbeat only extends the turn's life for as - long as the tool call is legitimately executing — it does not unbind - wedged tools. The 30-min gateway backstop remains for turns whose - agent loop itself stalls (no API call, no tool call in flight). + Keeps the gateway turn-inactivity watchdog (default 30 min) from abandoning a turn + whose tool runs silently. Wedged tools stay bounded by the tool layer's own timeouts. """ try: @@ -649,12 +596,7 @@ def _run_agent_tool_execution_middleware( block_msg, modified_args = _dispatch_pre_tool_call_hooks( function_name, final_args, - task_id=effective_task_id or "", - session_id=getattr(agent, "session_id", "") or "", - tool_call_id=tool_call_id or "", - turn_id=getattr(agent, "_current_turn_id", "") or "", - api_request_id=getattr(agent, "_current_api_request_id", "") - or "", + **tool_hook_ids(agent, effective_task_id, tool_call_id), middleware_trace=list(state["middleware_trace"]), ) if modified_args is not None: @@ -713,12 +655,8 @@ def _run_agent_tool_execution_middleware( _advance_start_order(_begin) - # Keep the gateway turn-inactivity watchdog from abandoning a turn - # whose tool call runs silently for longer than the inactivity - # timeout (#84491): stamp activity periodically while the tool is - # in flight, not just at start/completion. Both the sequential and - # the concurrent paths funnel through here, so a single heartbeat - # covers every tool. + # Heartbeat while the tool is in flight so the gateway inactivity watchdog + # doesn't abandon a silent-but-live turn (#84491); covers both executor paths. _hb_stop = threading.Event() _hb_thread = threading.Thread( target=_run_tool_activity_heartbeat, @@ -739,11 +677,7 @@ def _run_agent_tool_execution_middleware( function_name, relay_args, skip_relay=True, - task_id=effective_task_id or "", - session_id=getattr(agent, "session_id", "") or "", - tool_call_id=tool_call_id or "", - turn_id=getattr(agent, "_current_turn_id", "") or "", - api_request_id=getattr(agent, "_current_api_request_id", "") or "", + **tool_hook_ids(agent, effective_task_id, tool_call_id), ) request_args = ( request_result.payload @@ -759,11 +693,7 @@ def _run_agent_tool_execution_middleware( next_args if isinstance(next_args, dict) else request_args ), original_args=function_args, - task_id=effective_task_id or "", - session_id=getattr(agent, "session_id", "") or "", - tool_call_id=tool_call_id or "", - turn_id=getattr(agent, "_current_turn_id", "") or "", - api_request_id=getattr(agent, "_current_api_request_id", "") or "", + **tool_hook_ids(agent, effective_task_id, tool_call_id), ) result, _relay_args = relay_tools.execute( @@ -787,26 +717,20 @@ def _run_agent_tool_execution_middleware( ) -# How often the sequential-tool wait loop wakes to check for a user -# interrupt while the worker runs. Short enough that /stop or a redirect -# lands within ~1s even when the tool itself never polls is_interrupted(). +# Sequential wait-loop interrupt poll cadence: /stop lands within ~1s even when +# the tool never polls is_interrupted(). _SEQUENTIAL_INTERRUPT_POLL_SECONDS = 1.0 def _resolve_sequential_tool_timeout() -> float | None: """Deadline for one sequential tool call (#85125 Phase 2a). - ``timeouts.tools.sequential_call`` in config.yaml wins; when unset, the - sequential path inherits the concurrent batch deadline (same value, same - ``HERMES_CONCURRENT_TOOL_TIMEOUT_S`` legacy bridge) so the two executor - paths cannot drift apart by default. ``0``/negative disables the bound. + ``timeouts.tools.sequential_call`` wins; unset inherits the concurrent batch deadline + so the two paths can't drift. ``0``/negative disables the bound. - NOTE: this path deliberately does NOT use ``agent.deadline.run_bounded_sync``. - The sequential/concurrent executors extend their deadline dynamically while - a human approval prompt is open (``_ConcurrentToolAuthorizationGate`` - excluded seconds — a MUST-preserve invariant) and touch agent activity - mid-wait; the shared primitive is fixed-deadline by design. Simpler call - sites migrate onto the primitive; these two stay symmetric with each other. + Deliberately NOT ``agent.deadline.run_bounded_sync``: both executors extend their + deadline while an approval prompt is open (MUST-preserve), which the fixed-deadline + primitive can't express. """ from agent.deadline import resolve_timeout @@ -830,10 +754,8 @@ def _run_sequential_tool_execution_middleware( ) -> _ManagedToolResult: """Run one sequential call with the concurrent executor's deadline. - Interactive input tools such as ``clarify`` wait on a human. Their own - timeout (``agent.clarify_timeout``: default 3600s, or unlimited when - ``<= 0``) owns that wait. Applying the generic tool deadline here would - return ``tool_timeout`` while the prompt and worker stay active. + Interactive tools (``clarify``) own their wait via ``agent.clarify_timeout``; the + generic deadline would report ``tool_timeout`` while the prompt is still live. """ timeout_s = _resolve_sequential_tool_timeout() kwargs = { @@ -873,10 +795,8 @@ def _run_sequential_tool_execution_middleware( executor = DaemonThreadPoolExecutor(max_workers=1) future = executor.submit(propagate_context_to_thread(_run)) - # ``timeout_s`` disabled (None) still runs on the worker: the wait loop - # below is what makes a non-cooperative tool interruptible at all, so - # "no deadline" must not mean "no interrupt checks" (#86xxx class fix — - # sequential path previously blocked until the tool returned). + # Disabled timeout still runs on the worker: this wait loop is what makes a + # non-cooperative tool interruptible, so no deadline must not mean no interrupt checks. deadline = time.monotonic() + timeout_s if timeout_s is not None else None started = time.monotonic() timed_out = False @@ -918,9 +838,8 @@ def _run_sequential_tool_execution_middleware( ) except Exception: pass - # Give a cooperative tool a moment to notice its per-thread - # interrupt bit and return a real result (mirrors the concurrent - # path's 3s grace). + # Grace for a cooperative tool to notice its interrupt bit (mirrors the + # concurrent path's 3s). concurrent.futures.wait([future], timeout=3.0) if future.done() and not future.cancelled(): return future.result() @@ -1095,21 +1014,115 @@ def _begin_tool_execution( pass +def _append_finalized_tool_result( + agent, + messages: list, + *, + function_name: str, + function_args: dict, + function_result, + tool_call_id: str, + effective_task_id: str, + budget: BudgetConfig, + effect_disposition=None, +): + """Persist/spill, hint, wrap and append one tool result; flush the session DB. + + Returns ``(function_result, tool_message, risk_metadata)`` — ``function_result`` is the + persisted/hinted content — or ``None`` when the incremental flush failed (the caller + must stop the batch). + """ + if not _is_multimodal_tool_result(function_result): + function_result = maybe_persist_tool_result( + content=function_result, + tool_name=function_name, + tool_use_id=tool_call_id, + env=get_active_env(effective_task_id), + config=budget, + ) + _record_persisted_path_for_stub(agent, tool_call_id, function_result) + + subdir_hints = agent._subdirectory_hints.check_tool_call(function_name, function_args) + if subdir_hints: + if _is_multimodal_tool_result(function_result): + # Append the hint to the text summary part so the model still sees it; + # don't touch the image blocks. + _append_subdir_hint_to_multimodal(function_result, subdir_hints) + else: + function_result += subdir_hints + + # Unwrap _multimodal dicts to an OpenAI-style content list; text-only servers + # get a string-safe fallback so a rejected image result never poisons history. + _tool_content = agent._tool_result_content_for_active_model(function_name, function_result) + tool_message = make_tool_result_message( + function_name, + _tool_content, + tool_call_id, + effect_disposition=effect_disposition, + ) + messages.append(tool_message) + if not _flush_session_db_after_tool_progress( + agent, + messages, + stage=f"tool result {function_name}", + ): + return None + return function_result, tool_message, tool_message.get("_tool_output_risk") + + +def _emit_tool_completed_progress(agent, function_name: str, *, duration: float, is_error: bool, result) -> None: + """``tool.completed`` UI projection; downstream of the canonical append so resume + can reconstruct the result even if the UI bridge dies mid-projection.""" + if not agent.tool_progress_callback: + return + try: + agent.tool_progress_callback( + "tool.completed", function_name, None, None, + duration=duration, is_error=is_error, result=result, + ) + except Exception as cb_err: + logging.debug("Tool progress callback error: %s", cb_err) + + +def _emit_tool_complete_and_risk( + agent, *, function_name: str, function_args: dict, tool_call_id: str, result, risk_metadata, blocked: bool +) -> None: + """Fire ``tool_complete_callback`` (unless blocked) then the ``tool.output_risk`` projection.""" + if not blocked and agent.tool_complete_callback: + try: + display_args = _redact_tool_args_for_display(function_name, function_args) or function_args + agent.tool_complete_callback(tool_call_id, function_name, display_args, result) + except Exception as cb_err: + logging.debug("Tool complete callback error: %s", cb_err) + + if ( + risk_metadata is not None + and risk_metadata.get("risk") != "low" + and agent.tool_progress_callback + ): + try: + agent.tool_progress_callback( + "tool.output_risk", + function_name, + None, + None, + tool_call_id=tool_call_id, + risk_metadata=risk_metadata, + ) + except Exception as cb_err: + logging.debug("Tool output risk callback error: %s", cb_err) + + def execute_tool_calls_concurrent(agent, assistant_message, messages: list, effective_task_id: str, api_call_count: int = 0, *, finalize: bool = True) -> None: - """Execute multiple tool calls concurrently using a thread pool. + """Execute tool calls concurrently; results are appended in original call order. - Results are collected in the original tool-call order and appended to - messages so the API sees them in the expected sequence. - - ``finalize=False`` skips the end-of-batch aggregate budget enforcement - and /steer injection — used when this call is one segment of a larger - mixed batch and the segmented dispatcher owns the turn-end work. + ``finalize=False`` skips end-of-batch budget enforcement and /steer injection (the + segmented dispatcher owns turn-end work). """ tool_calls = assistant_message.tool_calls num_tools = len(tool_calls) - # Resolve the context-scaled tool-output budget once per turn (cheap, but - # avoids rebuilding it per result inside the loop below). + # Resolve the context-scaled tool-output budget once per turn, not per result. _tool_budget = _budget_for_agent(agent) # ── Pre-flight: interrupt check ────────────────────────────────── @@ -1145,17 +1158,11 @@ def execute_tool_calls_concurrent(agent, assistant_message, messages: list, effe ) return - # ── Parse args + pre-execution bookkeeping ─────────────────────── - # (tool call, resolved name, parsed args, middleware trace, parse error, - # tool-search scope block) + # ── Parse args + pre-execution bookkeeping ──────────────────────────── + # (tool call, name, args, middleware trace, parse error, tool-search scope block) parsed_calls = [] for tool_call in tool_calls: - function_name = tool_call.function.name - # Legacy tool-name aliases (2026-08 renames) — map BEFORE the - # agent-loop branches (todo_list etc. dispatch above the registry). - from model_tools import _LEGACY_TOOL_ALIASES as _lta - function_name = _lta.get(function_name, function_name) - + function_name = _canonical_tool_name(tool_call.function.name) function_args, malformed_args_result = _parse_tool_arguments( tool_call.function.arguments ) @@ -1173,45 +1180,9 @@ def execute_tool_calls_concurrent(agent, assistant_message, messages: list, effe ) continue - # ── Tool Search unwrap ──────────────────────────────────────── - # When the model invokes the tool_call bridge, peel it open so - # every downstream check (checkpointing, guardrails, plugin - # pre-tool-call hooks, the display/activity feed, the post-call - # callback) sees the underlying tool — not the bridge. This is - # the OpenClaw lesson: hooks must observe the real tool name. - # - # The original tool_call entry on ``tool_call.function`` is left - # untouched so the conversation transcript and the matching - # tool_call_id are preserved exactly as the model emitted them. - # - # Scope gate: the unwrap dispatches the underlying tool directly - # (bypassing the bridge branch in handle_function_call and its - # scope check), so we enforce session toolset scope HERE. A tool - # the session was not granted is rejected before any checkpoint, - # hook, or dispatch fires. - _ts_scope_block = None - try: - from tools import tool_search as _ts - if function_name == _ts.TOOL_CALL_NAME: - _underlying, _underlying_args, _err = _ts.resolve_underlying_call(function_args) - if not _err and _underlying: - if _underlying in _tool_search_scoped_names(agent): - # Validate before unwrapping: the generic bridge hides - # the concrete parameter schema from provider-native - # tool-call validation. - _probe_err = _ts.validate_deferred_call_args(_underlying, _underlying_args) - if _probe_err is not None: - _ts_scope_block = _probe_err - else: - function_name = _underlying - function_args = _underlying_args - else: - _ts_scope_block = ( - f"'{_underlying}' is not available in this session. " - "Use tool_search to find tools you can call." - ) - except Exception: - pass + function_name, function_args, _ts_scope_block = _unwrap_tool_search_call( + agent, function_name, function_args + ) parsed_calls.append( (tool_call, function_name, function_args, [], None, _ts_scope_block) @@ -1231,9 +1202,8 @@ def execute_tool_calls_concurrent(agent, assistant_message, messages: list, effe start_condition = threading.Condition() next_start_order = 0 - # Set once the batch is abandoned (deadline or interrupt) so a worker parked - # at the start-order gate exits immediately instead of waking up minutes - # later and dispatching a tool the turn has already reported as timed out. + # Set once the batch is abandoned so gate-parked workers exit instead of + # dispatching a tool the turn already reported as timed out. batch_abandoned = threading.Event() authorization_gate = _ConcurrentToolAuthorizationGate() @@ -1243,10 +1213,8 @@ def execute_tool_calls_concurrent(agent, assistant_message, messages: list, effe with start_condition: start_condition.notify_all() - # The gate bound must sit UNDER the batch deadline, otherwise the deadline - # fires first and the parked workers are still falsely reported as timed - # out without ever starting — the very bug this gate timeout fixes. A - # disabled deadline (None) keeps the stock bound rather than waiting forever. + # The gate bound must sit UNDER the batch deadline, else parked workers are falsely + # reported timed out without starting. A disabled deadline keeps the stock bound. def _start_order_gate_timeout(batch_timeout: float | None) -> float: if batch_timeout is None: return _START_ORDER_GATE_TIMEOUT_S @@ -1258,21 +1226,9 @@ def execute_tool_calls_concurrent(agent, assistant_message, messages: list, effe """Serialize dispatch by submit order. Returns False if abandoned.""" nonlocal next_start_order with start_condition: - # Bounded wait: a tool that wedges during its dispatch must not - # park every later-ordered worker forever. Without the timeout, - # one blocking dispatch starves the whole batch (the parked tools - # then get falsely reported as "timed out" by the batch deadline - # despite never having started) and the parked threads leak - # permanently after the batch is abandoned — f.cancel() cannot - # cancel running threads and nothing ever notifies the condition - # again. On expiry, proceed out of order: the worst case is - # interleaved approval prompts, strictly better than permanent - # starvation. The >= predicate (rather than ==) lets one worker's - # timeout-jump release every skipped worker immediately instead - # of each burning its own full timeout; max() keeps the counter - # monotonic when workers advance out of order. batch_abandoned - # short-circuits the wait so an abandoned batch releases its - # parked workers in milliseconds instead of one gate timeout. + # Bounded wait so one wedged dispatch can't starve/leak later-ordered workers; + # on expiry proceed out of order (interleaved prompts beat starvation). + # >= (not ==) releases every skipped worker at once; batch_abandoned short-circuits. in_order = start_condition.wait_for( lambda: next_start_order >= order or batch_abandoned.is_set(), timeout=( @@ -1319,16 +1275,13 @@ def execute_tool_calls_concurrent(agent, assistant_message, messages: list, effe start_order, ): """Worker function executed in a thread.""" - # Register this worker tid so the agent can fan out an interrupt - # to it — see AIAgent.interrupt(). Must happen first thing, and - # must be paired with discard + clear in the finally block. + # Register this worker tid for interrupt fan-out (AIAgent.interrupt()); must be + # first and paired with discard + clear in finally. _worker_tid = threading.current_thread().ident with agent._tool_worker_threads_lock: agent._tool_worker_threads.add(_worker_tid) - # Race: if the agent was interrupted between fan-out (which - # snapshotted an empty/earlier set) and our registration, apply - # the interrupt to our own tid now so is_interrupted() inside - # the tool returns True on the next poll. + # Race: interrupt may have fanned out before our registration; apply it + # to our own tid now. if agent._interrupt_requested: try: _ra()._set_interrupt( @@ -1338,18 +1291,15 @@ def execute_tool_calls_concurrent(agent, assistant_message, messages: list, effe ) except Exception: pass - # Set the activity callback on THIS worker thread so - # _wait_for_process (terminal commands) can fire heartbeats. - # The callback is thread-local; the main thread's callback - # is invisible to worker threads. + # Activity callback is thread-local; set it on THIS worker so + # _wait_for_process heartbeats fire. try: from tools.environments.base import set_activity_callback set_activity_callback(agent._touch_activity) except Exception: pass - # Approval/sudo callbacks (thread-local) and the agent turn's - # ContextVars are propagated by propagate_context_to_thread() at the - # submit site below (GHSA-qg5c-hvr5-hjgr, #13617). + # Approval/sudo callbacks and turn ContextVars are propagated by + # propagate_context_to_thread() at submit (GHSA-qg5c-hvr5-hjgr, #13617). start = time.time() tool_call_id = _pairing_tool_call_id(tool_call) blocked = False @@ -1408,11 +1358,8 @@ def execute_tool_calls_concurrent(agent, assistant_message, messages: list, effe blocked = managed.blocked dispatched = managed.dispatched except _BatchAbandoned: - # The batch was abandoned while we were parked at the start-order - # gate. The main thread already synthesized this tool's result - # (timeout/cancelled) and moved on, so write nothing: a late - # results[index] write, post_tool_call emit, or progress print - # would double-report a tool_call_id the turn already closed. + # Abandoned at the start-order gate: the main thread already synthesized + # this result, so write/emit nothing (would double-report the tool_call_id). logger.info( "tool %s abandoned at start-order gate; skipping dispatch", function_name, @@ -1474,19 +1421,14 @@ def execute_tool_calls_concurrent(agent, assistant_message, messages: list, effe middleware_trace, ) finally: - # Teardown advance: keep the counter moving for any later-ordered - # worker. Never let the abandonment signal escape from here — the - # worker is already unwinding and the turn owns the result. + # Teardown advance keeps later-ordered workers moving; never let the + # abandonment signal escape here. try: _advance_start() except _BatchAbandoned: pass - # Tear down worker-tid tracking. Clear any interrupt bit we may - # have set so the next task scheduled onto this recycled tid - # starts with a clean slate. This MUST be in a finally block - # because BaseException subclasses (CancelledError, KeyboardInterrupt) - # bypass ``except Exception`` and would otherwise leak the tid - # into _interrupted_threads, poisoning the recycled thread. + # Tear down tid tracking and clear any interrupt bit so a recycled tid starts + # clean. MUST be in finally: BaseException subclasses bypass ``except Exception``. with agent._tool_worker_threads_lock: agent._tool_worker_threads.discard(_worker_tid) try: @@ -1515,11 +1457,8 @@ def execute_tool_calls_concurrent(agent, assistant_message, messages: list, effe deadline = time.monotonic() + timeout_s if timeout_s is not None else None if runnable_calls: max_workers = _max_workers_for_tool_batch(runnable_calls) - # Daemon workers: an interrupted/timed-out batch is abandoned with - # shutdown(wait=False), but stdlib ThreadPoolExecutor workers are - # non-daemon and registered in concurrent.futures' atexit hook, - # which joins them unconditionally — so one wedged tool thread - # would block interpreter exit forever (multi-minute CLI exits). + # Daemon workers: stdlib ThreadPoolExecutor's atexit join would let one + # wedged tool thread block interpreter exit forever. from tools.daemon_pool import DaemonThreadPoolExecutor executor = DaemonThreadPoolExecutor(max_workers=max_workers) abandon_executor = False @@ -1527,9 +1466,8 @@ def execute_tool_calls_concurrent(agent, assistant_message, messages: list, effe for submit_index, (i, tc, name, args, scope_block) in enumerate( runnable_calls ): - # Propagate the agent turn's ContextVars (e.g. - # _approval_session_key) AND thread-local approval/sudo - # callbacks into the worker thread; clears callbacks on exit. + # Propagate turn ContextVars and thread-local approval/sudo + # callbacks into the worker; clears callbacks on exit. try: f = executor.submit( propagate_context_to_thread(_run_tool), @@ -1576,11 +1514,8 @@ def execute_tool_calls_concurrent(agent, assistant_message, messages: list, effe futures.append(f) future_to_index[f] = i - # Wait for all to complete with periodic heartbeats so the - # gateway's inactivity monitor doesn't kill us during long - # concurrent tool batches. Also check for user interrupts - # so we don't block indefinitely when the user sends /stop - # or a new message during concurrent tool execution. + # Wait with periodic heartbeats (gateway inactivity monitor) and + # interrupt checks (/stop or a new message). _conc_start = time.time() _interrupt_logged = False while True: @@ -1630,9 +1565,8 @@ def execute_tool_calls_concurrent(agent, assistant_message, messages: list, effe ) for f in not_done: f.cancel() - # Release gate-parked workers before the interrupt - # fan-out so none of them wakes up later and dispatches - # a tool this loop just reported as timed out. + # Release gate-parked workers before interrupt fan-out so none + # later dispatches a tool just reported as timed out. _abandon_batch() with agent._tool_worker_threads_lock: worker_tids = list(agent._tool_worker_threads) @@ -1643,11 +1577,8 @@ def execute_tool_calls_concurrent(agent, assistant_message, messages: list, effe pass break - # Check for interrupt — the per-thread interrupt signal - # already causes individual tools (terminal, execute_code) - # to abort, but tools without interrupt checks (web_search, - # read_file) will run to completion. Cancel any futures - # that haven't started yet so we don't block on them. + # Tools without interrupt checks (web_search, read_file) run to + # completion; cancel unstarted futures so we don't block on them. if agent._interrupt_requested: abandon_executor = True if not _interrupt_logged: @@ -1680,23 +1611,18 @@ def execute_tool_calls_concurrent(agent, assistant_message, messages: list, effe f"{len(not_done)} remaining: {', '.join(_still_running[:3])})" ) finally: - # Belt-and-braces: any exit from the wait loop that abandoned - # the batch must release gate-parked workers, including the - # exception path that never reaches the branches above. + # Any abandoning exit from the wait loop (including the exception + # path) must release gate-parked workers. if abandon_executor: _abandon_batch() - # On abandon (interrupt or deadline) we intentionally do NOT - # join hung workers: wait=False returns immediately and - # cancel_futures drops queued-but-unstarted work. A wedged tool - # thread is left running detached — the deliberate tradeoff vs. - # deadlocking the whole batch. Normal completion joins (wait=True). + # On abandon do NOT join hung workers: a wedged thread is left detached + # rather than deadlocking the batch. Normal completion joins. executor.shutdown( wait=not abandon_executor, cancel_futures=abandon_executor, ) finally: if spinner: - # Build a summary message for the spinner stop completed = sum(1 for r in results if r is not None) total_dur = sum(r[3] for r in results if r is not None) spinner.stop(f"⚡ {completed}/{num_tools} tools completed in {total_dur:.1f}s total") @@ -1710,10 +1636,8 @@ def execute_tool_calls_concurrent(agent, assistant_message, messages: list, effe blocked = False is_error = True progress_function_name = name - # A worker can finish and write results[i] in the window between the - # deadline snapshot (timed_out_indices, taken from not_done) and this - # loop. Prefer that real result over a fabricated timeout message — the - # tool genuinely succeeded, just slightly late. + # A worker may finish between the deadline snapshot and this loop; + # prefer its real result over a fabricated timeout. effect_disposition = None if i in timed_out_indices and r is None: suffix = f"{timeout_s:.1f}s" if timeout_s is not None else "the configured timeout" @@ -1799,9 +1723,8 @@ def execute_tool_calls_concurrent(agent, assistant_message, messages: list, effe result_preview = _err_text[:200] if len(_err_text) > 200 else _err_text logger.warning("Tool %s returned error (%.2fs): %s", function_name, tool_duration, result_preview) - # Track file-mutation outcome for the turn-end verifier. - # `blocked` calls never actually ran — don't let a guardrail - # block count as either a failure or a success. + # Track file-mutation outcome for the turn-end verifier; blocked calls + # never ran, so they count as neither failure nor success. if not blocked: try: agent._record_file_mutation_result( @@ -1819,62 +1742,27 @@ def execute_tool_calls_concurrent(agent, assistant_message, messages: list, effe agent._touch_activity(f"tool completed: {name} ({tool_duration:.1f}s){_status_suffix}") display_function_result = function_result - function_result = maybe_persist_tool_result( - content=function_result, - tool_name=name, - tool_use_id=tool_call_id, - env=get_active_env(effective_task_id), - config=_tool_budget, - ) if not _is_multimodal_tool_result(function_result) else function_result - _record_persisted_path_for_stub(agent, tool_call_id, function_result) - - subdir_hints = agent._subdirectory_hints.check_tool_call(name, args) - if subdir_hints: - if _is_multimodal_tool_result(function_result): - # Append the hint to the text summary part so the model - # still sees it; don't touch the image blocks. - _append_subdir_hint_to_multimodal(function_result, subdir_hints) - else: - function_result += subdir_hints - - # Unwrap _multimodal dicts to an OpenAI-style content list so any - # vision-capable provider receives [{type:text},{type:image_url}] - # rather than a raw Python dict. The Anthropic adapter already - # accepts content lists; vision-capable OpenAI-compatible servers - # (mlx-vlm, GPT-4o, …) accept image_url in tool messages natively. - # Text-only servers get a string-safe fallback here so a rejected - # image tool result never poisons canonical session history. - # String results pass through unchanged. - _tool_content = agent._tool_result_content_for_active_model(name, function_result) - tool_message = make_tool_result_message( - name, - _tool_content, - tool_call_id, - effect_disposition=effect_disposition, - ) - messages.append(tool_message) - risk_metadata = tool_message.get("_tool_output_risk") - if not _flush_session_db_after_tool_progress( + finalized = _append_finalized_tool_result( agent, messages, - stage=f"tool result {name}", - ): + function_name=name, + function_args=args, + function_result=function_result, + tool_call_id=tool_call_id, + effective_task_id=effective_task_id, + budget=_tool_budget, + effect_disposition=effect_disposition, + ) + if finalized is None: return + function_result, _tool_message, risk_metadata = finalized - # Every completion surface is downstream of the canonical append. If - # the UI bridge or process dies while projecting one of these events, - # resume can reconstruct the tool result that was already visible. - if not blocked and agent.tool_progress_callback: - try: - agent.tool_progress_callback( - "tool.completed", progress_function_name, None, None, - duration=tool_duration, is_error=is_error, - result=display_function_result, - ) - except Exception as cb_err: - logging.debug("Tool progress callback error: %s", cb_err) + if not blocked: + _emit_tool_completed_progress( + agent, progress_function_name, + duration=tool_duration, is_error=is_error, result=display_function_result, + ) - # Print cute message per tool if agent._should_emit_quiet_tool_messages(): cute_msg = _get_cute_tool_message_impl( name, args, tool_duration, result=display_function_result, @@ -1889,59 +1777,34 @@ def execute_tool_calls_concurrent(agent, assistant_message, messages: list, effe response_preview = _preview_str[:agent.log_prefix_chars] + "..." if len(_preview_str) > agent.log_prefix_chars else _preview_str print(f" ✅ Tool {i+1} completed in {tool_duration:.2f}s - {response_preview}") - if not blocked and agent.tool_complete_callback: - try: - display_args = _redact_tool_args_for_display(name, args) or args - agent.tool_complete_callback( - tool_call_id, name, display_args, display_function_result, - ) - except Exception as cb_err: - logging.debug("Tool complete callback error: %s", cb_err) + _emit_tool_complete_and_risk( + agent, + function_name=name, + function_args=args, + tool_call_id=tool_call_id, + result=display_function_result, + risk_metadata=risk_metadata, + blocked=blocked, + ) - if ( - risk_metadata is not None - and risk_metadata.get("risk") != "low" - and agent.tool_progress_callback - ): - try: - agent.tool_progress_callback( - "tool.output_risk", - name, - None, - None, - tool_call_id=tool_call_id, - risk_metadata=risk_metadata, - ) - except Exception as cb_err: - logging.debug("Tool output risk callback error: %s", cb_err) - - # ── Per-turn aggregate budget enforcement ───────────────────────── - # Keep /steer pending until the final post-budget drain below. The model - # cannot observe a partial batch, while an early drain can be discarded - # when aggregate budget enforcement replaces that tool result. + # ── Per-turn aggregate budget enforcement ────────────────────────── + # Keep /steer pending until the post-budget drain: an early drain could be + # discarded when budget enforcement replaces that tool result. num_tools = len(parsed_calls) if finalize and num_tools > 0: turn_tool_msgs = messages[-num_tools:] enforce_turn_budget(turn_tool_msgs, env=get_active_env(effective_task_id), config=_tool_budget) - # ── /steer injection ────────────────────────────────────────────── - # Append any pending user steer text to the last tool result so the - # agent sees it on its next iteration. Runs AFTER budget enforcement - # so the steer marker is never truncated. See steer() for details. + # ── /steer injection ──────────────────────────────────────────────── + # AFTER budget enforcement so the steer marker is never truncated; see steer(). if finalize and num_tools > 0: agent._apply_pending_steer_to_tool_results(messages, num_tools) def _append_cancelled_tool_results(messages: list, tool_calls, *, reason: str) -> None: - """Append a cancelled ``tool`` result for each call in ``tool_calls``. - - Used when a hard interrupt (KeyboardInterrupt / BaseException) aborts the - sequential executor mid-batch. Without this, the loop re-raises leaving the - assistant tool-call turn with no matching tool results — a message-role - alternation violation that malforms the next provider request. Mirrors the - cooperative-interrupt skip block and the concurrent path, both of which - already emit a result for every call_id. + """Append a cancelled ``tool`` result for each call so a hard interrupt never leaves + the assistant tool-call turn without matching results (role-alternation violation). """ for tc in tool_calls: name = getattr(getattr(tc, "function", None), "name", "") or "tool" @@ -1953,18 +1816,44 @@ def _append_cancelled_tool_results(messages: list, tool_calls, *, reason: str) - )) -def execute_tool_calls_sequential(agent, assistant_message, messages: list, effective_task_id: str, api_call_count: int = 0, *, finalize: bool = True) -> None: - """Execute tool calls sequentially (original behavior). Used for single calls or interactive tools. +def _start_quiet_tool_spinner(agent, function_name: str, function_args: dict, *, gate: bool = True): + """Start the quiet-mode kawaii spinner for one tool call, or return None. - ``finalize=False`` skips the end-of-batch aggregate budget enforcement - and /steer injection — used when this call is one segment of a larger - mixed batch and the segmented dispatcher owns the turn-end work. + ``gate=False`` skips ``_should_start_quiet_spinner`` (context-engine tools always spin). """ - # Resolve the context-scaled tool-output budget once per turn. + if not agent._should_emit_quiet_tool_messages(): + return None + if gate and not agent._should_start_quiet_spinner(): + return None + face = random.choice(KawaiiSpinner.get_waiting_faces()) + emoji = _get_tool_emoji(function_name) + display_args = _redact_tool_args_for_display(function_name, function_args) or function_args + preview = _build_tool_label(function_name, display_args) or function_name + spinner = KawaiiSpinner(f"{face} {emoji} {preview}", spinner_type='dots', print_fn=agent._print_fn) + spinner.start() + return spinner + + +def _finish_quiet_tool_spinner(agent, spinner, function_name: str, function_args: dict, tool_duration: float, result) -> None: + """Stop the spinner with the cute completion line, or print it when no spinner ran.""" + cute_msg = _get_cute_tool_message_impl(function_name, function_args, tool_duration, result=result) + if spinner: + spinner.stop(cute_msg) + elif agent._should_emit_quiet_tool_messages(): + agent._vprint(f" {cute_msg}") + + +def execute_tool_calls_sequential(agent, assistant_message, messages: list, effective_task_id: str, api_call_count: int = 0, *, finalize: bool = True) -> None: + """Execute tool calls sequentially (single calls or interactive tools). + + ``finalize=False`` skips end-of-batch budget enforcement and /steer injection (the + segmented dispatcher owns turn-end work). + """ + # Resolve the context-scaled tool-output budget once per turn, not per result. _tool_budget = _budget_for_agent(agent) - # Keep every runtime-tool branch on one bounded execution funnel without - # duplicating timeout policy across the branch-specific callbacks below. + # One bounded execution funnel for every runtime-tool branch; no duplicated + # timeout policy in the callbacks below. def _run_agent_tool_execution_middleware(agent, **kwargs): return _run_sequential_tool_execution_middleware(agent, **kwargs) @@ -1972,9 +1861,8 @@ def execute_tool_calls_sequential(agent, assistant_message, messages: list, effe tool_call_id = _pairing_tool_call_id(tool_call) if getattr(agent, "_incremental_persistence_failed", False): return - # SAFETY: check interrupt BEFORE starting each tool. - # If the user sent "stop" during a previous tool's execution, - # do NOT start any more tools -- skip them all immediately. + # SAFETY: check interrupt BEFORE each tool so a "stop" during the previous + # tool skips all remaining ones. if agent._interrupt_requested: remaining_calls = assistant_message.tool_calls[i-1:] if remaining_calls: @@ -2010,12 +1898,7 @@ def execute_tool_calls_sequential(agent, assistant_message, messages: list, effe return break - function_name = tool_call.function.name - # Legacy tool-name aliases (2026-08 renames) — map BEFORE the - # agent-loop branches (todo_list etc. dispatch above the registry). - from model_tools import _LEGACY_TOOL_ALIASES as _lta - function_name = _lta.get(function_name, function_name) - + function_name = _canonical_tool_name(tool_call.function.name) function_args, malformed_args_result = _parse_tool_arguments( tool_call.function.arguments ) @@ -2046,42 +1929,9 @@ def execute_tool_calls_sequential(agent, assistant_message, messages: list, effe return continue - # Tool Search unwrap — see execute_tool_calls_concurrent for full - # rationale, including the scope gate (the unwrap dispatches the - # underlying tool directly, so session toolset scope is enforced here). - _ts_scope_block: Optional[str] = None - try: - from tools import tool_search as _ts - if function_name == _ts.TOOL_CALL_NAME: - _underlying, _underlying_args, _err = _ts.resolve_underlying_call(function_args) - if not _err and _underlying: - if _underlying in _tool_search_scoped_names(agent): - # Validate before unwrapping: the generic bridge hides - # the concrete parameter schema from provider-native - # tool-call validation. - _probe_err = _ts.validate_deferred_call_args(_underlying, _underlying_args) - if _probe_err is not None: - # This path wraps _block_msg in {"error": ...} — - # flatten the probe payload to one plain string. - try: - _probe = json.loads(_probe_err) - _ts_scope_block = ( - f"{_probe.get('error', '')} Parameters schema: " - f"{json.dumps(_probe.get('parameters', {}), ensure_ascii=False)}. " - f"{_probe.get('hint', '')}" - ).strip() - except Exception: - _ts_scope_block = _probe_err - else: - function_name = _underlying - function_args = _underlying_args - else: - _ts_scope_block = ( - f"'{_underlying}' is not available in this session. " - "Use tool_search to find tools you can call." - ) - except Exception: - pass + function_name, function_args, _ts_scope_block = _unwrap_tool_search_call( + agent, function_name, function_args, flatten_probe=True + ) middleware_trace: list[dict[str, Any]] = [] _execution_blocked = False @@ -2089,14 +1939,17 @@ def execute_tool_calls_sequential(agent, assistant_message, messages: list, effe tool_start_time = time.time() - if function_name == "todo_list": + if function_name != "delegate_task" and function_name in INLINE_TOOL_EXECUTORS: + # Agent-level tools that need live AIAgent state; table shared with invoke_tool. + inline_executor = INLINE_TOOL_EXECUTORS[function_name] + inline_ctx = InlineToolContext( + effective_task_id=effective_task_id, + tool_call_id=tool_call_id, + messages=messages, + ) + def _execute(next_args: dict) -> Any: - from tools.todo_tool import todo_tool as _todo_tool - return _todo_tool( - todos=next_args.get("todos"), - merge=next_args.get("merge", False), - store=agent._todo_store, - ) + return inline_executor(agent, next_args, inline_ctx) function_result, function_args, middleware_trace, _execution_blocked, _execution_dispatched = _managed_values(_run_agent_tool_execution_middleware( agent, function_name=function_name, @@ -2109,290 +1962,7 @@ def execute_tool_calls_sequential(agent, assistant_message, messages: list, effe )) tool_duration = time.time() - tool_start_time if agent._should_emit_quiet_tool_messages(): - agent._vprint(f" {_get_cute_tool_message_impl('todo_list', function_args, tool_duration, result=function_result)}") - elif function_name == "message_agent": - # Bot Mode teammate DM (tools/bot_mode_dm.py) — injected, not - # registered: only a canonical Bot Chat session carries the - # schema, and the tool re-gates on the session title itself. - def _execute(next_args: dict) -> Any: - from tools.bot_mode_dm import message_agent_tool as _message_agent_tool - return _message_agent_tool( - target=next_args.get("target", ""), - message=next_args.get("message", ""), - task_id=effective_task_id, - agent=agent, - ) - function_result, function_args, middleware_trace, _execution_blocked, _execution_dispatched = _managed_values(_run_agent_tool_execution_middleware( - agent, - function_name=function_name, - function_args=function_args, - effective_task_id=effective_task_id, - tool_call_id=tool_call_id, - execute=_execute, - scope_block=_ts_scope_block, - display_index=i, - )) - tool_duration = time.time() - tool_start_time - if agent._should_emit_quiet_tool_messages(): - agent._vprint(f" {_get_cute_tool_message_impl('message_agent', function_args, tool_duration, result=function_result)}") - elif function_name == "session_search": - def _execute(next_args: dict) -> Any: - session_db = agent._get_session_db_for_recall() - if not session_db: - from hermes_state import format_session_db_unavailable - return json.dumps({"success": False, "error": format_session_db_unavailable()}) - from tools.session_search_tool import session_search as _session_search - return _session_search( - query=next_args.get("query", ""), - role_filter=next_args.get("role_filter"), - limit=next_args.get("limit", 3), - session_id=next_args.get("session_id"), - around_message_id=next_args.get("around_message_id"), - window=next_args.get("window", 5), - sort=next_args.get("sort"), - detail=next_args.get("detail", "adaptive"), - db=session_db, - current_session_id=agent.session_id, - ) - function_result, function_args, middleware_trace, _execution_blocked, _execution_dispatched = _managed_values(_run_agent_tool_execution_middleware( - agent, - function_name=function_name, - function_args=function_args, - effective_task_id=effective_task_id, - tool_call_id=tool_call_id, - execute=_execute, - scope_block=_ts_scope_block, - display_index=i, - )) - tool_duration = time.time() - tool_start_time - if agent._should_emit_quiet_tool_messages(): - agent._vprint(f" {_get_cute_tool_message_impl('session_search', function_args, tool_duration, result=function_result)}") - elif function_name == "memory": - def _execute(next_args: dict) -> Any: - target = next_args.get("target", "memory") - operations = next_args.get("operations") - from tools.memory_tool import memory_tool as _memory_tool - result = _memory_tool( - action=next_args.get("action"), - target=target, - content=next_args.get("content"), - old_text=next_args.get("old_text"), - operations=operations, - store=agent._memory_store, - ) - # Mirror successful built-in memory writes to external - # providers. All gating/op-expansion lives behind the manager - # interface (MemoryManager.notify_memory_tool_write). - if agent._memory_manager: - agent._memory_manager.notify_memory_tool_write( - result, - next_args, - build_metadata=lambda: agent._build_memory_write_metadata( - task_id=effective_task_id, - tool_call_id=tool_call_id, - ), - ) - return result - function_result, function_args, middleware_trace, _execution_blocked, _execution_dispatched = _managed_values(_run_agent_tool_execution_middleware( - agent, - function_name=function_name, - function_args=function_args, - effective_task_id=effective_task_id, - tool_call_id=tool_call_id, - execute=_execute, - scope_block=_ts_scope_block, - display_index=i, - )) - tool_duration = time.time() - tool_start_time - if agent._should_emit_quiet_tool_messages(): - agent._vprint(f" {_get_cute_tool_message_impl('memory', function_args, tool_duration, result=function_result)}") - elif function_name == "clarify": - def _execute(next_args: dict) -> Any: - from tools.clarify_tool import clarify_tool as _clarify_tool - return _clarify_tool( - question=next_args.get("question", ""), - choices=next_args.get("choices"), - multi_select=next_args.get("multi_select", False), - questions=next_args.get("questions"), - callback=agent.clarify_callback, - ) - function_result, function_args, middleware_trace, _execution_blocked, _execution_dispatched = _managed_values(_run_agent_tool_execution_middleware( - agent, - function_name=function_name, - function_args=function_args, - effective_task_id=effective_task_id, - tool_call_id=tool_call_id, - execute=_execute, - scope_block=_ts_scope_block, - display_index=i, - )) - tool_duration = time.time() - tool_start_time - if agent._should_emit_quiet_tool_messages(): - agent._vprint(f" {_get_cute_tool_message_impl('clarify', function_args, tool_duration, result=function_result)}") - elif function_name == "read_terminal": - def _execute(next_args: dict) -> Any: - from tools.read_terminal_tool import read_terminal_tool as _read_terminal_tool - return _read_terminal_tool( - start_line=next_args.get("start_line"), - count=next_args.get("count"), - callback=getattr(agent, "read_terminal_callback", None), - ) - function_result, function_args, middleware_trace, _execution_blocked, _execution_dispatched = _managed_values(_run_agent_tool_execution_middleware( - agent, - function_name=function_name, - function_args=function_args, - effective_task_id=effective_task_id, - tool_call_id=tool_call_id, - execute=_execute, - scope_block=_ts_scope_block, - display_index=i, - )) - tool_duration = time.time() - tool_start_time - if agent._should_emit_quiet_tool_messages(): - agent._vprint(f" {_get_cute_tool_message_impl('read_terminal', function_args, tool_duration, result=function_result)}") - elif function_name == "desktop_preview": - def _execute(next_args: dict) -> Any: - if (next_args.get("action") or "").strip() == "read": - from tools.read_preview_tool import read_preview_tool as _read_preview_tool - return _read_preview_tool( - start=next_args.get("start"), - count=next_args.get("count"), - callback=getattr(agent, "read_preview_callback", None), - ) - from tools.preview_tool import _handle_preview - return _handle_preview(next_args) - function_result, function_args, middleware_trace, _execution_blocked, _execution_dispatched = _managed_values(_run_agent_tool_execution_middleware( - agent, - function_name=function_name, - function_args=function_args, - effective_task_id=effective_task_id, - tool_call_id=tool_call_id, - execute=_execute, - scope_block=_ts_scope_block, - display_index=i, - )) - tool_duration = time.time() - tool_start_time - if agent._should_emit_quiet_tool_messages(): - agent._vprint(f" {_get_cute_tool_message_impl('desktop_preview', function_args, tool_duration, result=function_result)}") - elif function_name == "drive_preview": - def _execute(next_args: dict) -> Any: - from tools.drive_preview_tool import drive_preview_tool as _drive_preview_tool - return _drive_preview_tool( - action=next_args.get("action", ""), - ref=next_args.get("ref"), - selector=next_args.get("selector"), - text=next_args.get("text"), - key=next_args.get("key"), - submit=next_args.get("submit"), - amount=next_args.get("amount"), - to=next_args.get("to"), - limit=next_args.get("max"), - callback=getattr(agent, "drive_preview_callback", None), - ) - function_result, function_args, middleware_trace, _execution_blocked, _execution_dispatched = _managed_values(_run_agent_tool_execution_middleware( - agent, - function_name=function_name, - function_args=function_args, - effective_task_id=effective_task_id, - tool_call_id=tool_call_id, - execute=_execute, - scope_block=_ts_scope_block, - display_index=i, - )) - tool_duration = time.time() - tool_start_time - if agent._should_emit_quiet_tool_messages(): - agent._vprint(f" {_get_cute_tool_message_impl('drive_preview', function_args, tool_duration, result=function_result)}") - elif function_name == "annotate_preview": - def _execute(next_args: dict) -> Any: - from tools.annotate_preview_tool import annotate_preview_tool as _annotate_preview_tool - return _annotate_preview_tool( - action=next_args.get("action", "add"), - ref=next_args.get("ref"), - selector=next_args.get("selector"), - label=next_args.get("label"), - callback=getattr(agent, "drive_preview_callback", None), - ) - function_result, function_args, middleware_trace, _execution_blocked, _execution_dispatched = _managed_values(_run_agent_tool_execution_middleware( - agent, - function_name=function_name, - function_args=function_args, - effective_task_id=effective_task_id, - tool_call_id=tool_call_id, - execute=_execute, - scope_block=_ts_scope_block, - display_index=i, - )) - tool_duration = time.time() - tool_start_time - if agent._should_emit_quiet_tool_messages(): - agent._vprint(f" {_get_cute_tool_message_impl('annotate_preview', function_args, tool_duration, result=function_result)}") - elif function_name == "read_window_below": - def _execute(next_args: dict) -> Any: - from tools.read_window_tool import read_window_below_tool as _read_window_below_tool - return _read_window_below_tool( - callback=getattr(agent, "read_window_below_callback", None), - ) - function_result, function_args, middleware_trace, _execution_blocked, _execution_dispatched = _managed_values(_run_agent_tool_execution_middleware( - agent, - function_name=function_name, - function_args=function_args, - effective_task_id=effective_task_id, - tool_call_id=tool_call_id, - execute=_execute, - scope_block=_ts_scope_block, - display_index=i, - )) - tool_duration = time.time() - tool_start_time - if agent._should_emit_quiet_tool_messages(): - agent._vprint(f" {_get_cute_tool_message_impl('read_window_below', function_args, tool_duration, result=function_result)}") - elif function_name == "gui_tour": - def _execute(next_args: dict) -> Any: - from tools.tour_tool import tour_tool as _tour_tool - return _tour_tool( - action=next_args.get("action", ""), - surface=next_args.get("surface"), - selector=next_args.get("selector"), - title=next_args.get("title"), - text=next_args.get("text"), - side=next_args.get("side"), - steps=next_args.get("steps"), - step_index=next_args.get("step_index"), - callback=getattr(agent, "tour_callback", None), - ) - function_result, function_args, middleware_trace, _execution_blocked, _execution_dispatched = _managed_values(_run_agent_tool_execution_middleware( - agent, - function_name=function_name, - function_args=function_args, - effective_task_id=effective_task_id, - tool_call_id=tool_call_id, - execute=_execute, - scope_block=_ts_scope_block, - display_index=i, - )) - tool_duration = time.time() - tool_start_time - if agent._should_emit_quiet_tool_messages(): - agent._vprint(f" {_get_cute_tool_message_impl('gui_tour', function_args, tool_duration, result=function_result)}") - elif function_name == "setup_mcp": - def _execute(next_args: dict) -> Any: - from tools.setup_mcp_tool import setup_mcp_tool as _setup_mcp_tool - return _setup_mcp_tool( - server=next_args.get("server", ""), - action=next_args.get("action", "install"), - reason=next_args.get("reason", ""), - callback=getattr(agent, "setup_mcp_callback", None), - ) - function_result, function_args, middleware_trace, _execution_blocked, _execution_dispatched = _managed_values(_run_agent_tool_execution_middleware( - agent, - function_name=function_name, - function_args=function_args, - effective_task_id=effective_task_id, - tool_call_id=getattr(tool_call, "id", "") or "", - execute=_execute, - scope_block=_ts_scope_block, - display_index=i, - )) - tool_duration = time.time() - tool_start_time - if agent._should_emit_quiet_tool_messages(): - agent._vprint(f" {_get_cute_tool_message_impl('setup_mcp', function_args, tool_duration, result=function_result)}") + agent._vprint(f" {_get_cute_tool_message_impl(function_name, function_args, tool_duration, result=function_result)}") elif function_name == "delegate_task": _action_arg = str(function_args.get("action") or "").strip().lower() tasks_arg = function_args.get("tasks") @@ -2431,21 +2001,10 @@ def execute_tool_calls_sequential(agent, assistant_message, messages: list, effe finally: agent._delegate_spinner = None tool_duration = time.time() - tool_start_time - cute_msg = _get_cute_tool_message_impl('delegate_task', function_args, tool_duration, result=_delegate_result) - if spinner: - spinner.stop(cute_msg) - elif agent._should_emit_quiet_tool_messages(): - agent._vprint(f" {cute_msg}") + _finish_quiet_tool_spinner(agent, spinner, 'delegate_task', function_args, tool_duration, _delegate_result) elif agent._context_engine_tool_names and function_name in agent._context_engine_tool_names: # Context engine tools (lcm_grep, lcm_describe, lcm_expand, etc.) - spinner = None - if agent._should_emit_quiet_tool_messages(): - face = random.choice(KawaiiSpinner.get_waiting_faces()) - emoji = _get_tool_emoji(function_name) - display_args = _redact_tool_args_for_display(function_name, function_args) or function_args - preview = _build_tool_label(function_name, display_args) or function_name - spinner = KawaiiSpinner(f"{face} {emoji} {preview}", spinner_type='dots', print_fn=agent._print_fn) - spinner.start() + spinner = _start_quiet_tool_spinner(agent, function_name, function_args, gate=False) _ce_result = None try: def _execute(next_args: dict) -> Any: @@ -2466,22 +2025,11 @@ def execute_tool_calls_sequential(agent, assistant_message, messages: list, effe logger.error("context_engine.handle_tool_call raised for %s: %s", function_name, tool_error, exc_info=True) finally: tool_duration = time.time() - tool_start_time - cute_msg = _get_cute_tool_message_impl(function_name, function_args, tool_duration, result=_ce_result) - if spinner: - spinner.stop(cute_msg) - elif agent._should_emit_quiet_tool_messages(): - agent._vprint(f" {cute_msg}") + _finish_quiet_tool_spinner(agent, spinner, function_name, function_args, tool_duration, _ce_result) elif agent._memory_manager and agent._memory_manager.has_tool(function_name): # Memory provider tools (hindsight_retain, honcho_search, etc.) # These are not in the tool registry — route through MemoryManager. - spinner = None - if agent._should_emit_quiet_tool_messages() and agent._should_start_quiet_spinner(): - face = random.choice(KawaiiSpinner.get_waiting_faces()) - emoji = _get_tool_emoji(function_name) - display_args = _redact_tool_args_for_display(function_name, function_args) or function_args - preview = _build_tool_label(function_name, display_args) or function_name - spinner = KawaiiSpinner(f"{face} {emoji} {preview}", spinner_type='dots', print_fn=agent._print_fn) - spinner.start() + spinner = _start_quiet_tool_spinner(agent, function_name, function_args) _mem_result = None try: def _execute(next_args: dict) -> Any: @@ -2502,20 +2050,10 @@ def execute_tool_calls_sequential(agent, assistant_message, messages: list, effe logger.error("memory_manager.handle_tool_call raised for %s: %s", function_name, tool_error, exc_info=True) finally: tool_duration = time.time() - tool_start_time - cute_msg = _get_cute_tool_message_impl(function_name, function_args, tool_duration, result=_mem_result) - if spinner: - spinner.stop(cute_msg) - elif agent._should_emit_quiet_tool_messages(): - agent._vprint(f" {cute_msg}") - elif agent.quiet_mode: - spinner = None - if agent._should_emit_quiet_tool_messages() and agent._should_start_quiet_spinner(): - face = random.choice(KawaiiSpinner.get_waiting_faces()) - emoji = _get_tool_emoji(function_name) - display_args = _redact_tool_args_for_display(function_name, function_args) or function_args - preview = _build_tool_label(function_name, display_args) or function_name - spinner = KawaiiSpinner(f"{face} {emoji} {preview}", spinner_type='dots', print_fn=agent._print_fn) - spinner.start() + _finish_quiet_tool_spinner(agent, spinner, function_name, function_args, tool_duration, _mem_result) + else: + # Registry tools: post hook is owned by this executor (inner observer suppressed). + spinner = _start_quiet_tool_spinner(agent, function_name, function_args) if agent.quiet_mode else None _spinner_result = None try: def _execute(next_args: dict) -> Any: @@ -2579,9 +2117,8 @@ def execute_tool_calls_sequential(agent, assistant_message, messages: list, effe agent.interrupt("keyboard interrupt") except Exception: pass - # Emit a tool result for THIS call and every remaining call in - # the batch before re-raising, so the assistant tool-call turn - # is never left without matching tool results (alternation). + # Emit results for THIS and every remaining call before re-raising so + # the tool-call turn keeps matching results (alternation). _append_cancelled_tool_results( messages, assistant_message.tool_calls[i - 1:], @@ -2593,84 +2130,8 @@ def execute_tool_calls_sequential(agent, assistant_message, messages: list, effe logger.error("handle_function_call raised for %s: %s", function_name, tool_error, exc_info=True) finally: tool_duration = time.time() - tool_start_time - cute_msg = _get_cute_tool_message_impl(function_name, function_args, tool_duration, result=_spinner_result) - if spinner: - spinner.stop(cute_msg) - elif agent._should_emit_quiet_tool_messages(): - agent._vprint(f" {cute_msg}") - else: - try: - def _execute(next_args: dict) -> Any: - from model_tools import suppress_post_tool_call_hook - - with suppress_post_tool_call_hook(): - return _ra().handle_function_call( - function_name, - next_args, - effective_task_id, - tool_call_id=tool_call_id, - session_id=agent.session_id or "", - turn_id=getattr(agent, "_current_turn_id", "") or "", - api_request_id=getattr(agent, "_current_api_request_id", "") - or "", - enabled_tools=( - list(agent.valid_tool_names) - if agent.valid_tool_names - else None - ), - skip_pre_tool_call_hook=True, - skip_tool_request_middleware=True, - skip_tool_execution_middleware=True, - tool_request_middleware_trace=list(middleware_trace), - enabled_toolsets=getattr(agent, "enabled_toolsets", None), - disabled_toolsets=getattr(agent, "disabled_toolsets", None), - ) - - ( - function_result, - function_args, - middleware_trace, - _execution_blocked, - _execution_dispatched, - ) = _managed_values( - _run_agent_tool_execution_middleware( - agent, - function_name=function_name, - function_args=function_args, - effective_task_id=effective_task_id, - tool_call_id=tool_call_id, - execute=_execute, - scope_block=_ts_scope_block, - display_index=i, - middleware_trace=middleware_trace, - ) - ) - except KeyboardInterrupt: - _emit_cancelled_terminal_post_tool_call( - agent, - function_name=function_name, - function_args=function_args, - effective_task_id=effective_task_id, - tool_call_id=tool_call_id, - start_time=tool_start_time, - middleware_trace=list(middleware_trace), - ) - try: - agent.interrupt("keyboard interrupt") - except Exception: - pass - # Emit a tool result for THIS call and every remaining call in - # the batch before re-raising (see interactive branch above). - _append_cancelled_tool_results( - messages, - assistant_message.tool_calls[i - 1:], - reason="keyboard interrupt", - ) - raise - except Exception as tool_error: - function_result = f"Error executing tool '{function_name}': {tool_error}" - logger.error("handle_function_call raised for %s: %s", function_name, tool_error, exc_info=True) - tool_duration = time.time() - tool_start_time + if agent.quiet_mode: + _finish_quiet_tool_spinner(agent, spinner, function_name, function_args, tool_duration, _spinner_result) _execution_timed_out = isinstance( function_result, (_ToolTimeoutResult, _ToolCancelledResult) @@ -2688,13 +2149,9 @@ def execute_tool_calls_sequential(agent, assistant_message, messages: list, effe # Log tool errors to the persistent error log so [error] tags # in the UI always have a corresponding detailed entry on disk. _is_error_result, _ = _detect_tool_failure(function_name, function_result) - # The agent-runtime tools above (todo, session_search, memory, - # context-engine, memory-manager, clarify, delegate_task) are - # dispatched inline — they never reach handle_function_call, so the - # executor is the one that has to fire post_tool_call. For - # Every dispatch suppresses the inner handle_function_call observer so - # the executor owns one terminal event for this tool_call_id. This also - # prevents an abandoned timeout worker from reporting late success. + # Inline-dispatched runtime tools never reach handle_function_call, so the + # executor owns the one terminal post_tool_call per tool_call_id (the inner + # observer is suppressed); also stops an abandoned timeout worker reporting late. _executor_must_emit_post_hook = ( not _execution_blocked and not _execution_timed_out @@ -2726,10 +2183,8 @@ def execute_tool_calls_sequential(agent, assistant_message, messages: list, effe else: logger.info("tool %s completed (%.2fs, %d chars)", function_name, tool_duration, _result_len) - # Track file-mutation outcome for the turn-end verifier. See - # the concurrent path for the rationale; both paths must feed - # the same state so the footer reflects every tool call in the - # turn, not just the parallel ones. + # Track file-mutation outcome for the turn-end verifier; both paths feed + # the same state so the footer reflects every tool call. if not _execution_blocked: try: agent._record_file_mutation_result( @@ -2748,84 +2203,35 @@ def execute_tool_calls_sequential(agent, assistant_message, messages: list, effe logging.debug("Tool result (%d chars): %s", len(_log_result), _log_result) display_function_result = function_result - function_result = maybe_persist_tool_result( - content=function_result, - tool_name=function_name, - tool_use_id=tool_call_id, - env=get_active_env(effective_task_id), - config=_tool_budget, - ) if not _is_multimodal_tool_result(function_result) else function_result - _record_persisted_path_for_stub(agent, tool_call_id, function_result) - - # Discover subdirectory context files from tool arguments - subdir_hints = agent._subdirectory_hints.check_tool_call(function_name, function_args) - if subdir_hints: - if _is_multimodal_tool_result(function_result): - _append_subdir_hint_to_multimodal(function_result, subdir_hints) - else: - function_result += subdir_hints - - # Unwrap _multimodal dicts to an OpenAI-style content list - # (see parallel path for rationale). String results pass through. - _tool_content = agent._tool_result_content_for_active_model(function_name, function_result) - tool_message = make_tool_result_message( - function_name, - _tool_content, - tool_call_id, - effect_disposition="unknown" if _execution_timed_out else None, - ) - messages.append(tool_message) - risk_metadata = tool_message.get("_tool_output_risk") - if not _flush_session_db_after_tool_progress( + finalized = _append_finalized_tool_result( agent, messages, - stage=f"tool result {function_name}", - ): + function_name=function_name, + function_args=function_args, + function_result=function_result, + tool_call_id=tool_call_id, + effective_task_id=effective_task_id, + budget=_tool_budget, + effect_disposition="unknown" if _execution_timed_out else None, + ) + if finalized is None: return + function_result, _tool_message, risk_metadata = finalized - # UI completion/progress events are projections of the canonical tool - # row, never a competing in-memory authority. - if not _execution_blocked and agent.tool_progress_callback: - try: - agent.tool_progress_callback( - "tool.completed", function_name, None, None, - duration=tool_duration, is_error=_is_error_result, - result=display_function_result, - ) - except Exception as cb_err: - logging.debug("Tool progress callback error: %s", cb_err) - - if not _execution_blocked and agent.tool_complete_callback: - try: - display_args = ( - _redact_tool_args_for_display(function_name, function_args) - or function_args - ) - agent.tool_complete_callback( - tool_call_id, - function_name, - display_args, - display_function_result, - ) - except Exception as cb_err: - logging.debug("Tool complete callback error: %s", cb_err) - - if ( - risk_metadata is not None - and risk_metadata.get("risk") != "low" - and agent.tool_progress_callback - ): - try: - agent.tool_progress_callback( - "tool.output_risk", - function_name, - None, - None, - tool_call_id=tool_call_id, - risk_metadata=risk_metadata, - ) - except Exception as cb_err: - logging.debug("Tool output risk callback error: %s", cb_err) + if not _execution_blocked: + _emit_tool_completed_progress( + agent, function_name, + duration=tool_duration, is_error=_is_error_result, result=display_function_result, + ) + _emit_tool_complete_and_risk( + agent, + function_name=function_name, + function_args=function_args, + tool_call_id=tool_call_id, + result=display_function_result, + risk_metadata=risk_metadata, + blocked=_execution_blocked, + ) if not agent.quiet_mode and getattr(agent, "tool_progress_mode", "all") != "off": if agent.verbose_logging: @@ -2855,17 +2261,15 @@ def execute_tool_calls_sequential(agent, assistant_message, messages: list, effe return break - # ── Per-turn aggregate budget enforcement ───────────────────────── - # Keep /steer pending until the final post-budget drain below. The model - # only receives this batch after all calls finish, and an early drain can - # be discarded when aggregate budget enforcement replaces a tool result. + # ── Per-turn aggregate budget enforcement ────────────────────────── + # Keep /steer pending until the post-budget drain: an early drain could be + # discarded when budget enforcement replaces a tool result. num_tools_seq = len(assistant_message.tool_calls) if finalize and num_tools_seq > 0: enforce_turn_budget(messages[-num_tools_seq:], env=get_active_env(effective_task_id), config=_tool_budget) - # ── /steer injection ────────────────────────────────────────────── - # See _execute_tool_calls_parallel for the rationale. Same hook, - # applied to sequential execution as well. + # ── /steer injection ──────────────────────────────────────────────── + # See the concurrent path for rationale. if finalize and num_tools_seq > 0: agent._apply_pending_steer_to_tool_results(messages, num_tools_seq) @@ -2873,27 +2277,13 @@ def execute_tool_calls_sequential(agent, assistant_message, messages: list, effe def execute_tool_calls_segmented(agent, assistant_message, messages: list, effective_task_id: str, api_call_count: int = 0, segments=None) -> None: - """Execute a mixed tool-call batch as ordered parallel/sequential segments. + """Execute a mixed batch as ordered parallel/sequential segments. - ``segments`` is the ``(kind, calls)`` plan from - ``_plan_tool_batch_segments``: maximal contiguous runs of parallel-safe - calls execute on the concurrent path, barrier calls on the sequential - path, strictly in the model's original call order. Because segments are - contiguous, every tool result is still appended one-per-call in emission - order and no call ever starts before an earlier barrier finishes — - identical ordering and side-effect boundaries to fully-sequential - execution, with I/O parallelism recovered inside the safe runs. - - Turn-end work (aggregate budget enforcement + /steer injection) is done - once here for the WHOLE batch; the per-segment executor calls run with - ``finalize=False`` so a multi-segment turn cannot multiply the budget or - truncate a steer marker. - - Interrupt semantics: each segment executor already checks - ``agent._interrupt_requested`` up front and appends a cancelled/skipped - result per call, so an interrupt during segment *k* drains segments - *k+1..n* without executing them while preserving one result per - tool_call_id. + ``segments`` is the ``(kind, calls)`` plan from ``_plan_tool_batch_segments``; + contiguous segments preserve per-call result order and barrier boundaries exactly + as fully-sequential execution. Turn-end work (budget + /steer) runs once here; + segment executors run with ``finalize=False``. Each segment executor checks the + interrupt flag up front, so an interrupt drains later segments with one result per call. """ from types import SimpleNamespace diff --git a/tests/agent/test_intent_ack_continuation.py b/tests/agent/test_intent_ack_continuation.py index 2a1934beb7..cee992866f 100644 --- a/tests/agent/test_intent_ack_continuation.py +++ b/tests/agent/test_intent_ack_continuation.py @@ -15,7 +15,6 @@ from types import SimpleNamespace from typing import Union from agent.agent_runtime_helpers import ( - intent_ack_continuation_enabled, intent_ack_continuation_mode, looks_like_codex_intermediate_ack, ) @@ -72,13 +71,6 @@ def test_missing_attr_defaults_to_auto(): assert intent_ack_continuation_mode(bare_codex) == "codex_only" -def test_enabled_is_mode_not_off(): - assert intent_ack_continuation_enabled(_agent(True, "chat_completions")) is True - assert intent_ack_continuation_enabled(_agent("auto", "codex_responses")) is True - assert intent_ack_continuation_enabled(_agent("auto", "chat_completions")) is False - assert intent_ack_continuation_enabled(_agent(False, "codex_responses")) is False - - # ── detector: workspace requirement ───────────────────────────────────────── diff --git a/tests/run_agent/test_run_agent.py b/tests/run_agent/test_run_agent.py index 00ad41174c..f4ed1db3c0 100644 --- a/tests/run_agent/test_run_agent.py +++ b/tests/run_agent/test_run_agent.py @@ -2343,20 +2343,6 @@ class TestConcurrentToolExecution: assert post_calls[0]["status"] == "ok" assert post_calls[0]["result"] == '{"intercepted":true}' - def test_agent_runtime_post_hook_ownership_predicate_covers_agent_tools(self, agent): - """Sequential and concurrent agent-level paths share post-hook ownership.""" - from agent.agent_runtime_helpers import agent_runtime_owns_post_tool_hook - - for tool_name in ("todo_list", "session_search", "memory", "clarify", "delegate_task"): - assert agent_runtime_owns_post_tool_hook(agent, tool_name) is True - - agent._context_engine_tool_names = {"context_query"} - assert agent_runtime_owns_post_tool_hook(agent, "context_query") is True - - agent._memory_manager = SimpleNamespace(has_tool=lambda name: name == "memory_extra") - assert agent_runtime_owns_post_tool_hook(agent, "memory_extra") is True - assert agent_runtime_owns_post_tool_hook(agent, "web_search") is False - def test_blocked_memory_tool_does_not_reset_counter(self, agent, monkeypatch): """Blocked memory tool should not reset the nudge counter.""" agent._turns_since_memory = 5