diff --git a/agent/agent_init.py b/agent/agent_init.py index 69d814f47dcc..95161f8cdee5 100644 --- a/agent/agent_init.py +++ b/agent/agent_init.py @@ -1952,8 +1952,12 @@ def init_agent( # parent_session_id chain, no `name #N` renumber). See #38763 and # agent/conversation_compression.py. Consumed by compress_context(), not the # compressor, so it rides on the agent. + # Default True must match DEFAULT_CONFIG["compression"]["in_place"] + # (#38763). default=False here previously flipped agents into rotation + # mode whenever the merged config omitted the key (partial configs, + # load_config failure → {}), re-arming the pre-lease drift abort. compression_in_place = is_truthy_value( - _compression_cfg.get("in_place"), default=False + _compression_cfg.get("in_place"), default=True ) codex_app_server_auto_compaction = str( _compression_cfg.get("codex_app_server_auto", "native") or "native" diff --git a/agent/auxiliary_client.py b/agent/auxiliary_client.py index b31d90e78804..2f59cb417736 100644 --- a/agent/auxiliary_client.py +++ b/agent/auxiliary_client.py @@ -4381,6 +4381,7 @@ def _try_main_agent_model_fallback( failed_provider: str, task: str = None, reason: str = "error", + failed_model: Optional[str] = None, ) -> Tuple[Optional[Any], Optional[str], str]: """Last-resort fallback to the user's main agent provider + model. @@ -4389,8 +4390,23 @@ def _try_main_agent_model_fallback( layer: if nothing the user asked for can serve the request, try the main chat model before giving up. - Skips when the failed provider already IS the main provider (no point - retrying the same backend that just failed). + ``failed_model`` narrows the same-provider skip to the exact + (provider, model) pair that just failed, mirroring + :func:`_try_configured_fallback_chain`. This matters for self-hosted / + custom endpoints serving several models behind one provider label: the + aux compression model timing out says nothing about the health of the + main agent model deployed on the same URL (real incident: aux + ``glm-5.2`` hung and timed out while main ``macaron-v1-venti`` on the + identical endpoint was serving 448K-token turns fine — the + provider-label skip discarded the one fallback that would have worked). + + - Model-specific runtime failures (timeout, connection, rate limit, + model-incompatible, invalid response) pass ``failed_model``: skip the + main model only when it IS the exact model that failed. + - Provider-wide failures (auth 401, payment 402) and legacy callers + leave ``failed_model`` as None, keeping the whole-provider skip — + the shared credentials/account are broken, so the main model on the + same provider cannot help either. Returns: (client, model, provider_label) or (None, None, "") if no fallback. @@ -4407,9 +4423,23 @@ def _try_main_agent_model_fallback( if not main_provider or not main_model or main_provider.lower() in {"auto", ""}: return None, None, "" - skip = (failed_provider or "").lower().strip() - if main_provider.lower() == skip: - # The thing that failed IS the main model — nothing to fall back to. + # Identity + scope semantics owned by agent.backend_identity (#72468): + # model-scoped failures skip only the exact deployment that failed; + # provider-wide failures (no failed_model) skip the credential surface. + from agent.backend_identity import ( + BackendIdentity, + FailureScope, + should_skip_candidate, + ) + + skip_model = (failed_model or "").strip().lower() or None + if should_skip_candidate( + BackendIdentity.build(provider=main_provider, model=main_model), + BackendIdentity.build(provider=failed_provider, model=skip_model), + FailureScope.MODEL if skip_model else FailureScope.CREDENTIAL, + ): + # The thing that failed IS the main model (or the failure was + # provider-wide) — nothing to fall back to. return None, None, "" if _is_provider_unhealthy(main_provider): _log_skip_unhealthy(main_provider, task) @@ -4519,6 +4549,7 @@ def _try_configured_fallback_chain( task: str, failed_provider: str, reason: str = "error", + failed_model: Optional[str] = None, ) -> Tuple[Optional[Any], Optional[str], str]: """Try user-configured fallback_chain for a specific auxiliary task. @@ -4526,6 +4557,25 @@ def _try_configured_fallback_chain( entry in order. Each entry must have at least ``provider``; ``model``, ``base_url``, and ``api_key`` are optional. + ``failed_model`` narrows the skip check to the exact (provider, model) + pair that just failed, rather than the whole provider. Without it every + entry sharing the failed provider is skipped (the original behaviour). + Callers pass it only when a sibling model on the same provider could + plausibly recover: + + - Model-specific runtime failures (timeout, connection, rate limit, + model-incompatible, invalid response) pass ``failed_model`` so a + chain that intentionally lists several models under the same provider + — e.g. two more NVIDIA NIM models after the primary NIM model times + out — is not skipped wholesale. Only the exact model that failed is + skipped; the siblings still run instead of jumping straight to the + main-agent-model safety net. + - Provider-wide failures (auth 401, payment 402) and "no client could + be built" callers leave ``failed_model`` as None, keeping the whole + provider skipped — the shared credentials/account behind every model + on that provider are broken, so a sibling can't help and the + main-agent-model safety net should be reached instead. + Returns: (client, model, provider_label) or (None, None, "") if no fallback. """ @@ -4537,7 +4587,24 @@ def _try_configured_fallback_chain( if not chain or not isinstance(chain, list): return None, None, "" - skip = failed_provider.lower().strip() + skip_model = (failed_model or "").strip().lower() or None + # Identity + scope semantics owned by agent.backend_identity (#59561, + # #72468): a failed_model means the failure was model-scoped (timeout / + # connection / rate limit) — only the exact deployment is skipped; no + # failed_model means provider-wide (auth/payment) — the whole credential + # surface is skipped. + from agent.backend_identity import ( + BackendIdentity, + FailureScope, + should_skip_candidate, + ) + + failed_ident = BackendIdentity.build( + provider=failed_provider, model=skip_model, + ) + failure_scope = ( + FailureScope.MODEL if skip_model else FailureScope.CREDENTIAL + ) tried = [] min_ctx = _task_minimum_context_length(task) @@ -4545,9 +4612,20 @@ def _try_configured_fallback_chain( if not isinstance(entry, dict): continue fb_provider = str(entry.get("provider", "")).strip() - if not fb_provider or fb_provider.lower() == skip: + if not fb_provider: continue - fb_model = str(entry.get("model", "")).strip() or None + fb_model_raw = str(entry.get("model", "")).strip() + if should_skip_candidate( + BackendIdentity.build( + provider=fb_provider, + model=fb_model_raw, + base_url=str(entry.get("base_url") or ""), + ), + failed_ident, + failure_scope, + ): + continue + fb_model = fb_model_raw or None label = f"fallback_chain[{i}]({fb_provider})" @@ -8355,6 +8433,15 @@ def call_llm( logger.info("Auxiliary %s: %s on %s (%s), trying fallback", task or "call", reason, resolved_provider, first_err) + # Narrow the configured-chain skip to the exact model that + # failed ONLY for model-specific failures. Auth (401) and + # payment (402) errors are provider-wide — the credentials or + # account behind every model on that provider are the same — so + # a sibling model can't recover; keep skipping the whole + # provider so the main-agent-model safety net is still reached. + _chain_failed_model = ( + None if reason in ("auth error", "payment error") else final_model + ) # Fallback order (#26882, #26803): # 1. User-configured fallback_chain (per-task) if set # 2. For auto: top-level main fallback_providers/fallback_model @@ -8363,7 +8450,8 @@ def call_llm( fb_client, fb_model, fb_label = (None, None, "") if is_auto: fb_client, fb_model, fb_label = _try_configured_fallback_chain( - task, resolved_provider or "auto", reason=reason) + task, resolved_provider or "auto", reason=reason, + failed_model=_chain_failed_model) if fb_client is None: fb_client, fb_model, fb_label = _try_main_fallback_chain( task, resolved_provider or "auto", reason=reason) @@ -8372,10 +8460,12 @@ def call_llm( resolved_provider, task, reason=reason) else: fb_client, fb_model, fb_label = _try_configured_fallback_chain( - task, resolved_provider or "auto", reason=reason) + task, resolved_provider or "auto", reason=reason, + failed_model=_chain_failed_model) if fb_client is None: fb_client, fb_model, fb_label = _try_main_agent_model_fallback( - resolved_provider, task, reason=reason) + resolved_provider, task, reason=reason, + failed_model=_chain_failed_model) if fb_client is not None: fb_resp = _call_fallback_candidate_sync( @@ -8939,6 +9029,15 @@ async def async_call_llm( logger.info("Auxiliary %s (async): %s on %s (%s), trying fallback", task or "call", reason, resolved_provider, first_err) + # Narrow the configured-chain skip to the exact model that + # failed ONLY for model-specific failures. Auth (401) and + # payment (402) errors are provider-wide — the credentials or + # account behind every model on that provider are the same — so + # a sibling model can't recover; keep skipping the whole + # provider so the main-agent-model safety net is still reached. + _chain_failed_model = ( + None if reason in ("auth error", "payment error") else final_model + ) # Fallback order (#26882, #26803): # 1. User-configured fallback_chain (per-task) if set # 2. For auto: top-level main fallback_providers/fallback_model @@ -8947,7 +9046,8 @@ async def async_call_llm( fb_client, fb_model, fb_label = (None, None, "") if is_auto: fb_client, fb_model, fb_label = _try_configured_fallback_chain( - task, resolved_provider or "auto", reason=reason) + task, resolved_provider or "auto", reason=reason, + failed_model=_chain_failed_model) if fb_client is None: fb_client, fb_model, fb_label = _try_main_fallback_chain( task, resolved_provider or "auto", reason=reason) @@ -8956,10 +9056,12 @@ async def async_call_llm( resolved_provider, task, reason=reason) else: fb_client, fb_model, fb_label = _try_configured_fallback_chain( - task, resolved_provider or "auto", reason=reason) + task, resolved_provider or "auto", reason=reason, + failed_model=_chain_failed_model) if fb_client is None: fb_client, fb_model, fb_label = _try_main_agent_model_fallback( - resolved_provider, task, reason=reason) + resolved_provider, task, reason=reason, + failed_model=_chain_failed_model) if fb_client is not None: # Convert sync fallback client to async diff --git a/agent/backend_identity.py b/agent/backend_identity.py new file mode 100644 index 000000000000..7a7e9efb6bfe --- /dev/null +++ b/agent/backend_identity.py @@ -0,0 +1,204 @@ +"""Single owner for backend identity and failure-scoped skip decisions. + +Every fallback / dedup / skip / quarantine decision in Hermes ultimately asks +one question: **"is this candidate the same backend as the one that failed, +along the axis that failure invalidated?"** Before this module, that +question was re-implemented inline at six call sites across four subsystems, +each comparing whatever string was locally convenient (provider label, +provider+model, base_url+model, ...). Each incident fixed one site while the +others kept the bug: #22548 (same-shim aliases), #70893 (xai-oauth vs xai — +same host, distinct credential), #59561 (aux chain skipped sibling models), +#72468 (aux main-model safety net, same bug three weeks later), #62984 / +#54250 / #57584 (dedup ignoring base_url strands multi-endpoint pools). + +The root insight: "provider" conflates three independent identity axes, and +each failure class invalidates a different one: + +* **credential surface** — auth 401 / payment 402 kill everything sharing the + credential (every model, every host reached with that key/token). +* **endpoint** — DNS failure / connection refused kill everything behind the + URL, regardless of model or credential. +* **model deployment** — timeout / overload / rate limit / model-incompatible + kill ONE model's deployment. A sibling model behind the same URL is an + independent deployment (real incident: aux ``glm-5.2`` hung and timed out + while main ``macaron-v1-venti`` on the identical endpoint was serving + 448K-token turns). + +Call sites should build :class:`BackendIdentity` values, classify the failure +with :func:`classify_failure_scope`, and ask :func:`should_skip_candidate`. +Do not re-implement any comparison inline — extend THIS module instead. +""" + +from __future__ import annotations + +import logging +from dataclasses import dataclass +from enum import Enum +from typing import Optional + +logger = logging.getLogger(__name__) + + +class FailureScope(Enum): + """Which identity axis a failure invalidates.""" + + #: Timeout, overload/429, connection blip, model-incompatible, invalid + #: response: evidence against ONE model deployment only. + MODEL = "model" + #: Auth 401 / payment 402: evidence against the shared credential — + #: every model reached with it is equally dead. + CREDENTIAL = "credential" + #: DNS / connection-refused / unreachable host: evidence against the + #: endpoint — every model behind the URL is equally dead. + ENDPOINT = "endpoint" + + +#: Reason strings already used by auxiliary_client's except-chain, mapped to +#: scopes. Unknown reasons default to MODEL — the least-invalidating scope — +#: so an unrecognized failure never over-skips viable candidates. +_REASON_SCOPES = { + "auth error": FailureScope.CREDENTIAL, + "payment error": FailureScope.CREDENTIAL, + "rate limit": FailureScope.MODEL, + "model incompatible with route": FailureScope.MODEL, + "invalid provider response": FailureScope.MODEL, + "connection error": FailureScope.MODEL, + "timeout": FailureScope.MODEL, +} + + +def classify_failure_scope(reason: Optional[str]) -> FailureScope: + """Map a human-readable failure reason to the identity axis it kills.""" + return _REASON_SCOPES.get((reason or "").strip().lower(), FailureScope.MODEL) + + +def _norm_provider(value: Optional[str]) -> str: + return (value or "").strip().lower() + + +def _norm_model(value: Optional[str]) -> str: + return (value or "").strip().lower() + + +def _norm_base_url(value: Optional[str]) -> str: + return (value or "").strip().rstrip("/").lower() + + +@dataclass(frozen=True) +class BackendIdentity: + """Normalized identity of one (provider, model, endpoint) deployment. + + Empty fields mean "unknown" — comparisons treat an unknown axis as + non-distinguishing (it can neither prove sameness nor difference on its + own; the remaining axes decide). + """ + + provider: str = "" + model: str = "" + base_url: str = "" + + @classmethod + def build( + cls, + provider: Optional[str] = None, + model: Optional[str] = None, + base_url: Optional[str] = None, + ) -> "BackendIdentity": + return cls( + provider=_norm_provider(provider), + model=_norm_model(model), + base_url=_norm_base_url(base_url), + ) + + +def _both_first_class(a: BackendIdentity, b: BackendIdentity) -> bool: + """True when both providers are distinct registered first-class providers. + + Two different registry providers have distinct credential surfaces even + when they share an inference host (xai-oauth vs xai, openai-codex vs + openai-api) — #70893. Custom/shim aliases are NOT in the registry, so + two aliases pointing at one URL still count as the same backend (#22548). + """ + if not a.provider or not b.provider or a.provider == b.provider: + return False + try: + from hermes_cli.auth import PROVIDER_REGISTRY + + return a.provider in PROVIDER_REGISTRY and b.provider in PROVIDER_REGISTRY + except Exception: + return False + + +def same_credential_surface(a: BackendIdentity, b: BackendIdentity) -> bool: + """Do two identities share the credential a 401/402 just invalidated? + + Conservative on purpose: an unprovable axis must answer "different" + (try the candidate — worst case one wasted RTT) rather than "same" + (skip — worst case stranded failover). Two distinct custom labels at + one URL may carry different per-entry api_keys, so a shared URL alone + never proves a shared credential; it is only used as a weak signal + when a provider label is missing entirely. + """ + if a.provider and b.provider: + # Same label = same configured credential. Different labels = + # different credential config (first-class registry providers + # explicitly so — #70893; custom entries can each carry their own + # api_key, so sameness is unprovable and we must not skip). + return a.provider == b.provider + # Provider unknown on a side: same explicit URL is the best signal left. + return bool(a.base_url and a.base_url == b.base_url) + + +def same_endpoint(a: BackendIdentity, b: BackendIdentity) -> bool: + """Do two identities sit behind the endpoint that just went unreachable?""" + if a.base_url and b.base_url: + return a.base_url == b.base_url + # An unknown base_url inherits the provider default → same provider + # label implies the same default endpoint. + return bool(a.provider and a.provider == b.provider) + + +def same_deployment(a: BackendIdentity, b: BackendIdentity) -> bool: + """Are these the exact same model deployment (the thing a timeout kills)? + + Provider+model must match; the base_url axis distinguishes only when BOTH + sides carry an explicit URL (#62984: same provider+model on two different + explicit URLs is two deployments — a pool). A side with an unknown URL + inherits the provider default and cannot prove difference. + """ + if not (a.provider and b.provider and a.provider == b.provider): + # Same-host different-label shims: same URL + same model IS the same + # deployment even when the alias labels differ (#22548) — unless both + # labels are first-class registry providers (#70893). + if ( + a.base_url + and a.base_url == b.base_url + and a.model + and a.model == b.model + and not _both_first_class(a, b) + ): + return True + return False + if not (a.model and b.model and a.model == b.model): + return False + if a.base_url and b.base_url and a.base_url != b.base_url: + return False # distinct explicit endpoints — a pool, not a dup + return True + + +def should_skip_candidate( + candidate: BackendIdentity, + failed: BackendIdentity, + scope: FailureScope = FailureScope.MODEL, +) -> bool: + """THE skip predicate: would trying ``candidate`` just repeat the failure? + + True when the candidate is the same backend as ``failed`` along the axis + ``scope`` says the failure invalidated. Every fallback/dedup/skip site + must call this instead of comparing labels inline. + """ + if scope is FailureScope.CREDENTIAL: + return same_credential_surface(candidate, failed) + if scope is FailureScope.ENDPOINT: + return same_endpoint(candidate, failed) + return same_deployment(candidate, failed) diff --git a/agent/chat_completion_helpers.py b/agent/chat_completion_helpers.py index 15161ea11d4d..40fdaa57fe1f 100644 --- a/agent/chat_completion_helpers.py +++ b/agent/chat_completion_helpers.py @@ -440,26 +440,56 @@ def _dispatch_nonstreaming_api_request(agent, api_kwargs: dict, *, make_client): def should_use_direct_api_call(agent) -> bool: - """Whether a cron OpenAI-wire request should skip the interrupt worker. + """Whether an OpenAI-wire request should skip the interrupt worker. - Issue #62151 is specific to OpenRouter's chat-completions path inside the - gateway cron thread stack. Keep native/Codex/Bedrock/MoA transports on their - established workers: their cancellation and client ownership differ, and - the report provides no evidence that those paths share the pre-HTTP wedge. + Two nested-pool contexts wedge before the socket opens when the request + is pushed onto yet another daemon worker thread: + + - Gateway cron turns (#62151): gateway asyncio loop → cron thread → + interrupt worker. Fixed by running inline. + - Delegated children (#60203): gateway loop → async-delegation executor + (module-lifetime daemon pool) → per-child timeout executor → interrupt + worker. Same fingerprint after multi-day gateway uptime — children hang + at their FIRST API call with zero stale-detector output (the worker + never reaches dispatch), all providers, restart cures it. The cron fix + originally excluded delegation "for lack of evidence"; #60203 is that + evidence. + + Running inline drops the deepest thread layer (whose only job is + interactive-interrupt responsiveness). Interrupts still work: the inline + path registers ``agent._active_request_abort``, which ``interrupt()`` + invokes cross-thread to shut the active sockets — the same mechanism the + async-delegation stall monitor (#72227) relies on. + + Keep native/Codex/Bedrock/MoA transports on their established workers: + their cancellation and client ownership differ. """ - return ( - getattr(agent, "platform", None) == "cron" - and getattr(agent, "api_mode", None) == "chat_completions" - and getattr(agent, "provider", None) != "moa" - ) + if getattr(agent, "api_mode", None) != "chat_completions": + return False + if getattr(agent, "provider", None) == "moa": + return False + if getattr(agent, "platform", None) == "cron": + return True + # Delegated child (delegate_task sync or background) — detected via the + # execution ContextVar set by _run_single_child, with the agent's own + # platform stamp as a fallback for callers that bypass the runner. + try: + from agent.delegation_context import is_delegated_child_context + + if is_delegated_child_context(): + return True + except Exception: + pass + return getattr(agent, "platform", None) == "subagent" def direct_api_call(agent, api_kwargs: dict): """Run a non-streaming LLM call inline on the conversation thread. - Used when ``should_use_direct_api_call`` is True. Skips the interrupt worker - (whose only job is interactive-interrupt responsiveness, which this context - does not have) so the nested-pool deadlock (#62151) cannot occur. Because the + Used when ``should_use_direct_api_call`` is True (cron turns and + delegated children). Skips the interrupt worker (whose only job is + interactive-interrupt responsiveness, which these contexts do not have) + so the nested-pool deadlock (#62151, #60203) cannot occur. Because the request runs in-flight normally, the per-request OpenAI client's own httpx timeout (provider ``request_timeout_seconds`` / ``HERMES_API_TIMEOUT``) bounds a genuinely hung provider — the same bound interactive calls already rely on. @@ -470,7 +500,7 @@ def direct_api_call(agent, api_kwargs: dict): request_client_lock = threading.Lock() def _abort_active_request(reason: str) -> None: - """Abort the inline request from cron's watchdog/interrupt thread.""" + """Abort the inline request from a watchdog/interrupt thread.""" with request_client_lock: request_client = request_client_holder["client"] if request_client is not None: @@ -1521,45 +1551,6 @@ def _fallback_entry_key(fb: dict) -> tuple[str, str, str]: ) -def _fallback_entry_is_same_backend_by_base_url( - *, - current_provider: str, - fb_provider: str, - current_base_url: str, - fb_base_url: str, - current_model: str, - fb_model: str, -) -> bool: - """True when base_url+model identity means the fallback is the same backend. - - Issue #22548: two ``custom_providers`` aliases that point at the same shim - URL with the same model must be skipped, or failover loops on the dead - backend. First-class providers that share a host while using different - auth (``xai-oauth`` vs ``xai``, ``openai-codex`` vs ``openai-api``) are - distinct credential surfaces — skipping them strands configured failover - when primary and fallback reuse the same model slug on that host. - """ - if not ( - fb_base_url - and current_base_url - and fb_base_url == current_base_url - and fb_model == current_model - ): - return False - if fb_provider == current_provider: - return True - try: - from hermes_cli.auth import PROVIDER_REGISTRY - - # Both sides are registered first-class providers → different auth - # identities even when the inference host matches. Allow failover. - if current_provider in PROVIDER_REGISTRY and fb_provider in PROVIDER_REGISTRY: - return False - except Exception: - pass - return True - - def _fallback_entry_unavailable_without_network(agent, fb: dict) -> Optional[str]: """Return a skip reason for fallback entries known to be unusable locally.""" fb_provider = (fb.get("provider") or "").strip().lower() @@ -1645,33 +1636,28 @@ def try_activate_fallback(agent, reason: "FailoverReason | None" = None) -> bool ) return agent._try_activate_fallback(reason) - # Skip entries that resolve to the current (provider, model) — falling - # back to the same backend that just failed loops the failure. Compare - # base_url too so two distinct custom_providers entries pointing at the - # same shim/proxy URL also dedup. See issue #22548. Do NOT treat - # first-class providers that share a host (xai-oauth vs xai) as the same - # backend — they use different credentials. - current_provider = (getattr(agent, "provider", "") or "").strip().lower() - current_model = (getattr(agent, "model", "") or "").strip() - current_base_url = str(getattr(agent, "base_url", "") or "").rstrip("/").lower() - fb_base_url_for_dedup = (fb.get("base_url") or "").strip().rstrip("/").lower() - if fb_provider == current_provider and fb_model == current_model: + # Skip entries that resolve to the same backend that just failed — + # falling back to it loops the failure. Identity semantics (which axes + # distinguish two backends, shim aliases, first-class credential + # surfaces, multi-endpoint pools) are owned by agent.backend_identity — + # see #22548, #70893, #62984. Do not re-implement comparisons here. + from agent.backend_identity import BackendIdentity, should_skip_candidate + + current_ident = BackendIdentity.build( + provider=getattr(agent, "provider", ""), + model=getattr(agent, "model", ""), + base_url=str(getattr(agent, "base_url", "") or ""), + ) + fb_ident = BackendIdentity.build( + provider=fb_provider, + model=fb_model, + base_url=(fb.get("base_url") or ""), + ) + if should_skip_candidate(fb_ident, current_ident): logger.warning( - "Fallback skip: chain entry %s/%s matches current provider/model", - fb_provider, fb_model, - ) - return agent._try_activate_fallback(reason) - if _fallback_entry_is_same_backend_by_base_url( - current_provider=current_provider, - fb_provider=fb_provider, - current_base_url=current_base_url, - fb_base_url=fb_base_url_for_dedup, - current_model=current_model, - fb_model=fb_model, - ): - logger.warning( - "Fallback skip: chain entry base_url %s matches current backend", - fb_base_url_for_dedup, + "Fallback skip: chain entry %s/%s resolves to the same backend " + "as the current one (%s)", + fb_provider, fb_model, current_ident.base_url or current_ident.provider, ) return agent._try_activate_fallback(reason) diff --git a/agent/conversation_compression.py b/agent/conversation_compression.py index d2df6f25a39f..0f56f9a92c43 100644 --- a/agent/conversation_compression.py +++ b/agent/conversation_compression.py @@ -1402,7 +1402,10 @@ def compress_context( # `name #N` renumber, no contextvar/env/logging re-sync, no memory/context- # engine session-switch. The conversation keeps one durable id for life, # eliminating the session-rotation bug cluster. Default True (2107b86024). - in_place = bool(getattr(agent, "compression_in_place", False)) + # Default True matches DEFAULT_CONFIG / #38763. A missing attribute must + # NOT fall back to rotation mode — that re-enables the pre-lease drift + # path and can wedge busy sessions that never set the flag. + in_place = bool(getattr(agent, "compression_in_place", True)) # Set True once the in-place DB write actually completes (the DB block can # raise and skip it). Surfaced to the gateway via agent._last_compaction_in_place. compacted_in_place = False @@ -1712,6 +1715,13 @@ def compress_context( # non-destructive — pre-compaction rows are soft-archived (active=0, # compacted=1), stay searchable and recoverable, so snapshot/durable # drift cannot lose data there and must not abort compaction. + # + # When durable DID grow, ADOPT it and continue rather than aborting. + # Aborting returned the stale snapshot unchanged, so busy sessions + # (memory review / shared session_id writers) stayed permanently + # behind the DB: every /compress and auto-compress saw + # "changed before lease acquisition", surfaced as the misleading + # "No changes from compression", and never reclaimed tokens. if not in_place and _lock_db is not None and _lock_sid: durable_loader = getattr( type(_lock_db), "get_messages_as_conversation", None @@ -1719,16 +1729,19 @@ def compress_context( if callable(durable_loader): durable_parent = durable_loader(_lock_db, _lock_sid) if isinstance(durable_parent, list) and len(durable_parent) > len(messages): - logger.warning( - "compression aborted: session=%s changed before lease " - "acquisition; preserving newer durable messages", + logger.info( + "compression: session=%s grew before lease " + "(%d → %d msgs); adopting durable snapshot", _lock_sid, + len(messages), + len(durable_parent), ) - _release_lock() - existing_prompt = getattr(agent, "_cached_system_prompt", None) - if not existing_prompt: - existing_prompt = agent._build_system_prompt(system_message) - return messages, existing_prompt + messages = durable_parent + _pre_msg_count = len(messages) + # Token estimate was for the stale snapshot; clear it so + # the compressor re-derives from the adopted transcript + # instead of under-counting the newly visible rows. + approx_tokens = 0 # Notify external memory provider before compression discards context. # The provider's on_pre_compress() may return a string of insights it diff --git a/agent/conversation_loop.py b/agent/conversation_loop.py index 7e0475a1ffae..3f350c3ec8d2 100644 --- a/agent/conversation_loop.py +++ b/agent/conversation_loop.py @@ -5497,6 +5497,14 @@ def run_conversation( args_preview = raw_args[:200] if isinstance(raw_args, str) else repr(raw_args)[:200] logging.debug("Tool call: %s with args: %s...", tc.function.name, args_preview) + # Uniquify duplicate tool-call ids BEFORE any downstream + # consumer (validation error paths, dispatch, history build, + # Responses item-id derivation). Models that reuse one id for + # different calls in a batch otherwise lose the later call's + # result: the pre-API sanitizer keeps only the first + # call/result pair per id. See _uniquify_tool_call_ids. + agent._uniquify_tool_call_ids(assistant_message.tool_calls) + # Validate tool call names - detect model hallucinations # Repair mismatched tool names before validating for tc in assistant_message.tool_calls: @@ -6328,7 +6336,28 @@ def run_conversation( ". No fallback providers configured.") ) - final_response = "(empty)" + # Deliver a labeled reasoning excerpt instead of a bare + # "(empty)" when the model DID think but never produced + # visible text. This is delivery-only: the persisted + # assistant message above keeps the "(empty)" sentinel + # (its replay semantics prevent empty-response loops), + # and raw chain-of-thought is never promoted to a normal + # answer earlier in the ladder — prefill continuation, + # empty-content retries, and provider fallback all run + # first. Only at this terminal, where the alternative is + # returning nothing, is showing the model's own reasoning + # (clearly labeled as such) strictly more useful. + # Idea credit: PR #48795 (@ligl0325). + if reasoning_text: + final_response = ( + "⚠️ The model produced only internal reasoning and " + "no final answer, despite retries" + + (" and fallback" if agent._fallback_chain else "") + + ". Its last reasoning, which may contain the " + "answer:\n\n" + reasoning_preview + ) + else: + final_response = "(empty)" break # Reset retry counter/signature on successful content diff --git a/agent/error_classifier.py b/agent/error_classifier.py index 33c2f5458560..e629e7b7af9e 100644 --- a/agent/error_classifier.py +++ b/agent/error_classifier.py @@ -159,6 +159,14 @@ _RATE_LIMIT_PATTERNS = [ "throttlingexception", "too many concurrent requests", "servicequotaexceededexception", + # Generic throttle prefix — Bedrock (and some proxies) surface throttling + # as "Throttling error: Too many tokens, please wait before trying + # again." Without this entry the message falls through to the + # context-overflow list (which contains "too many tokens") and the retry + # loop compresses a healthy session instead of backing off. Matched + # BEFORE _CONTEXT_OVERFLOW_PATTERNS in the message-only path, so the + # throttle wins. (port of anomalyco/opencode#37848's exclusion guard) + "throttling", ] # Patterns that indicate provider-side overload, NOT a per-credential rate @@ -212,6 +220,12 @@ _PAYLOAD_TOO_LARGE_PATTERNS = [ "request entity too large", "payload too large", "error code: 413", + # Anthropic's structured 413 error type. Normally arrives with an HTTP + # 413 status (handled by the status path), but aggregators/proxies can + # re-wrap it into a plain message with no status attribute — route it to + # the same compression recovery. (port of anomalyco/opencode#37848) + "request_too_large", + "request exceeds the maximum size", ] # Image-size patterns. Matched against 400 bodies (not 413) because most @@ -298,6 +312,10 @@ _CONTEXT_OVERFLOW_PATTERNS = [ "max input token", "input token", "exceeds the maximum number of input tokens", + # Together/Fireworks-style: "Input length 131393 exceeds the maximum + # allowed input length of 131040 tokens." No other pattern in this list + # matches that wording. (port of anomalyco/opencode#37848) + "maximum allowed input length", ] # Model not found patterns diff --git a/agent/gemini_native_adapter.py b/agent/gemini_native_adapter.py index b4f6e6386e7d..bb53e32b2cec 100644 --- a/agent/gemini_native_adapter.py +++ b/agent/gemini_native_adapter.py @@ -163,6 +163,42 @@ _FREE_TIER_GUIDANCE = ( ) +def is_standard_key_auth_error( + status: int, error_message: str, reason: str = "" +) -> bool: + """Return True when a Gemini 401 indicates Google rejected the key TYPE. + + Google began rejecting unrestricted legacy "Standard" Google Cloud API + keys on the Gemini API on June 19, 2026, and ALL Standard keys stop + working in September 2026. The rejection surfaces as a misleading 401 + telling the user to supply an OAuth 2 access token ("Request had invalid + authentication credentials. Expected OAuth 2 access token, login cookie + or other valid authentication credential."), optionally carrying + ``google.rpc.ErrorInfo`` reason ``ACCESS_TOKEN_TYPE_UNSUPPORTED``. + + Scoped narrowly so a plain bad key (reason ``API_KEY_INVALID``, + "API key not valid") keeps its existing message. + """ + if status != 401: + return False + if reason == "ACCESS_TOKEN_TYPE_UNSUPPORTED": + return True + return "expected oauth 2 access token" in (error_message or "").lower() + + +_STANDARD_KEY_GUIDANCE = ( + "\n\nGoogle Gemini rejected this API key's type — you do NOT need OAuth. " + "Google began rejecting legacy 'Standard' Google Cloud keys for the " + "Gemini API on June 19, 2026, and all Standard keys stop working in " + "September 2026. Open https://aistudio.google.com/api-keys, check the " + "key's type and status, and create a replacement Gemini API key (or, as " + "a temporary bridge, restrict the Standard key to " + "generativelanguage.googleapis.com). Then update GEMINI_API_KEY / " + "GOOGLE_API_KEY in ~/.hermes/.env and restart your session. " + "Details: https://ai.google.dev/gemini-api/docs/api-key" +) + + class GeminiAPIError(Exception): """Error shape compatible with Hermes retry/error classification.""" @@ -824,6 +860,12 @@ def gemini_http_error( if status == 429 and is_free_tier_quota_error(err_message or body_text): message = message + _FREE_TIER_GUIDANCE + # Legacy "Standard" Google Cloud key rejection (June 19, 2026 onward) -> + # Google's raw 401 misleadingly tells the user to use OAuth. Append the + # actual fix (mint a new Gemini API key in AI Studio). + if is_standard_key_auth_error(status, err_message or body_text, reason): + message = message + _STANDARD_KEY_GUIDANCE + return GeminiAPIError( message, code=code, diff --git a/agent/gemini_schema.py b/agent/gemini_schema.py index b0985422fbb1..665fd79a37e4 100644 --- a/agent/gemini_schema.py +++ b/agent/gemini_schema.py @@ -2,6 +2,7 @@ from __future__ import annotations +import math from typing import Any, Dict # Gemini's ``FunctionDeclaration.parameters`` field accepts the ``Schema`` @@ -76,15 +77,31 @@ def sanitize_gemini_schema(schema: Any) -> Dict[str, Any]: # Gemini's Schema validator requires every ``enum`` entry to be a string, # even when the parent ``type`` is ``integer`` / ``number`` / ``boolean``. - # OpenAI / OpenRouter / Anthropic accept typed enums (e.g. Discord's - # ``auto_archive_duration: {type: integer, enum: [60, 1440, 4320, 10080]}``), - # so we only drop the ``enum`` when it would collide with Gemini's rule. - # Keeping ``type: integer`` plus the human-readable description gives the - # model enough guidance; the tool handler still validates the value. + # Preserve those constraints by stringifying scalar values while keeping + # the declared type intact; Gemini uses the strings as schema metadata and + # still emits typed tool arguments at runtime. enum_val = cleaned.get("enum") type_val = cleaned.get("type") if isinstance(enum_val, list) and type_val in {"integer", "number", "boolean"}: - if any(not isinstance(item, str) for item in enum_val): + stringified = [] + for item in enum_val: + if isinstance(item, str): + value = item + elif isinstance(item, bool): + value = "true" if item else "false" + elif ( + isinstance(item, (int, float)) + and not isinstance(item, bool) + and math.isfinite(item) + ): + value = str(item) + else: + continue + if value not in stringified: + stringified.append(value) + if stringified: + cleaned["enum"] = stringified + else: cleaned.pop("enum", None) # Gemini validates ``required`` strictly against the same node's diff --git a/agent/redact.py b/agent/redact.py index ebca1ae75f14..bff23934da6c 100644 --- a/agent/redact.py +++ b/agent/redact.py @@ -175,6 +175,85 @@ _YAML_ASSIGN_RE = re.compile( re.IGNORECASE | re.MULTILINE, ) +# Word-boundary validation for the mixed/lowercase key patterns above +# (_CFG_DOTTED_RE, _CFG_ANCHORED_RE, _YAML_ASSIGN_RE). +# +# Those key classes allow arbitrary alphanumeric affixes around the secret +# keyword so real key names like ``client_secret``, ``clientSecret``, and +# ``s3.secret-key`` match. The side effect: ordinary prose/document words that +# merely CONTAIN a keyword also matched — ``Secretary: J.Smith`` (secret), +# ``tokenizer: cl100k_base`` (token), ``author=Smith`` (auth) — mangling +# legitimate content on the surfaces that run these passes (browser snapshots, +# log lines, kanban summaries, CLI-echoed command output). Ported from +# nearai/ironclaw#6129, where the same substring false positive ("Secretary of +# the Treasury" matching the ``secret`` marker) scrubbed legitimate tool +# results from the replayed transcript and sent the model into a re-fetch +# loop. +# +# A keyword occurrence only counts when it sits at a word boundary within the +# key: at the key's edge, next to a non-letter (``_ - . 3``), or at a +# camelCase transition (``clientSecret``, ``secretKey``, ``APIToken``). A +# trailing plural ``s`` is treated as part of the keyword (``secrets:``, +# ``tokens:``). Common concatenated compounds keep matching via explicit +# alternatives (``authtoken`` ngrok, ``authkey`` tailscale, ``secretkey`` +# minio, ``apikey``). Embedded occurrences inside a larger word +# (``secretary``, ``tokenizer``, ``authored``, ``credentialing``) no longer +# match. ALL-CAPS keys keep the legacy embedded matching (``MYTOKEN=…``) — an +# all-caps key is almost never prose, the same rationale as _ENV_ASSIGN_RE. +_KEY_KEYWORD_RE = re.compile( + r"(?:api|auth|access|refresh|session|secret)[ _.\-]?(?:key|token)" + r"|token|secret|passwd|password|credential|auth", + re.IGNORECASE, +) + + +def _is_word_start(s: str, i: int) -> bool: + """True if position ``i`` in ``s`` begins a word (not mid-word).""" + if i == 0: + return True + prev, cur = s[i - 1], s[i] + if not prev.isalpha(): + return True + if cur.isupper() and prev.islower(): + return True # camelCase: clientSecret + # Acronym run ending: APIToken — the 'T' begins a new word when it is + # followed by lowercase while the preceding run is uppercase. + if cur.isupper() and prev.isupper() and i + 1 < len(s) and s[i + 1].islower(): + return True + return False + + +def _is_word_end(s: str, j: int, *, allow_plural: bool = True) -> bool: + """True if position ``j`` (exclusive end) in ``s`` ends a word.""" + if j >= len(s): + return True + cur = s[j] + if not cur.isalpha(): + return True + if cur.isupper() and s[j - 1].islower(): + return True # camelCase continuation: secretKey + if allow_plural and cur in "sS": + return _is_word_end(s, j + 1, allow_plural=False) + return False + + +def _key_has_secret_keyword(key: str) -> bool: + """True if ``key`` contains a secret keyword at a word boundary. + + Post-match validator for _CFG_DOTTED_RE / _CFG_ANCHORED_RE / + _YAML_ASSIGN_RE hits — rejects prose words that merely embed a keyword + (``secretary``, ``tokenizer``, ``authored``). Safe to call with the + _ENV_ASSIGN_RE key too: all-caps keys short-circuit to the legacy + embedded-match behavior. + """ + letters = [c for c in key if c.isalpha()] + if letters and all(c.isupper() for c in letters): + return True # legacy all-caps behavior (MYTOKEN=…) + for m in _KEY_KEYWORD_RE.finditer(key): + if _is_word_start(key, m.start()) and _is_word_end(key, m.end()): + return True + return False + # JSON field patterns: "apiKey": "value", "token": "value", etc. _JSON_KEY_NAMES = r"(?:api_?[Kk]ey|token|secret|password|access_token|refresh_token|auth_token|bearer|secret_value|raw_secret|secret_input|key_material)" _JSON_FIELD_RE = re.compile( @@ -614,6 +693,13 @@ def redact_sensitive_text( # prose/log contexts (issue #2852): ``KEY=os.getenv('X')``. if _ENV_LOOKUP_VALUE_RE.match(value): return m.group(0) + # Keyword must sit at a word boundary within the key — + # ``author=Smith`` / ``press.secretary=…`` are prose, not + # credentials (ported from nearai/ironclaw#6129). All-caps + # keys (the _ENV_ASSIGN_RE shape) short-circuit to legacy + # embedded matching inside the helper. + if not _key_has_secret_keyword(name): + return m.group(0) return f"{name}={quote}{_mask_token(value)}{quote}" text = _ENV_ASSIGN_RE.sub(_redact_env, text) # Lowercase/dotted config keys (issue #16413). Skip URLs entirely — @@ -647,6 +733,11 @@ def redact_sensitive_text( # not a leaked secret value. if _ENV_LOOKUP_VALUE_RE.match(value): return m.group(0) + # Keyword must sit at a word boundary within the key — + # ``Secretary: J.Smith`` / ``tokenizer: cl100k_base`` are + # document text, not credentials (nearai/ironclaw#6129). + if not _key_has_secret_keyword(key): + return m.group(0) return f"{key}{sep}{_mask_token(value)}" text = _YAML_ASSIGN_RE.sub(_redact_yaml, text) diff --git a/agent/shell_hooks.py b/agent/shell_hooks.py index 80453bc37139..5699e4d9eb65 100644 --- a/agent/shell_hooks.py +++ b/agent/shell_hooks.py @@ -100,6 +100,7 @@ emitted by each built-in hook site. child_role – role string of the child agent child_summary – summary of the child's work child_status – exit status string (e.g. "success", "error") + tool_call_history – redacted tool name/input summary/byte counts/status list duration_ms – wall-clock time of the child run in milliseconds """ diff --git a/agent/skill_utils.py b/agent/skill_utils.py index df0f933317fc..eea78d6a07c0 100644 --- a/agent/skill_utils.py +++ b/agent/skill_utils.py @@ -50,7 +50,7 @@ EXCLUDED_SKILL_DIRS = frozenset( SKILL_SUPPORT_DIRS = frozenset(("references", "templates", "assets", "scripts")) -def is_excluded_skill_path(path) -> bool: +def is_excluded_skill_path(path, *, root: Optional[Path] = None) -> bool: """True if *path* should be skipped by active skill scanners. Use this on every ``SKILL.md`` path produced by direct ``rglob`` scans to @@ -66,11 +66,11 @@ def is_excluded_skill_path(path) -> bool: from pathlib import PurePath parts = PurePath(str(path)).parts return any(part in EXCLUDED_SKILL_DIRS for part in parts) or is_skill_support_path( - path + path, root=root ) -def is_skill_support_path(path) -> bool: +def is_skill_support_path(path, *, root: Optional[Path] = None) -> bool: """True if *path* is under a support dir of an actual skill root. ``references/``, ``templates/``, ``assets/``, and ``scripts/`` are @@ -92,6 +92,8 @@ def is_skill_support_path(path) -> bool: if part not in SKILL_SUPPORT_DIRS or idx == 0: continue skill_root = Path(*parts[:idx]) + if root is not None and not path_obj.is_absolute(): + skill_root = root / skill_root if (skill_root / "SKILL.md").exists(): return True return False diff --git a/agent/subagent_lifecycle.py b/agent/subagent_lifecycle.py new file mode 100644 index 000000000000..b85d84d9e194 --- /dev/null +++ b/agent/subagent_lifecycle.py @@ -0,0 +1,533 @@ +"""Public, plugin-safe lifecycle API for delegated Hermes subagents. + +This module deliberately exposes immutable contracts, not ``AIAgent`` objects. +It is the supported boundary for plugins that need to supervise fresh child +sessions; plugins must obtain it from ``PluginContext.subagent_lifecycle``. +""" + +from __future__ import annotations + +import contextvars +import dataclasses +import enum +import hashlib +import hmac +import json +import math +import secrets +import threading +import time +from contextlib import contextmanager +from concurrent.futures import Future, ThreadPoolExecutor, TimeoutError +from typing import Any, Callable, Mapping, Optional + + +PUBLIC_CONTRACT_VERSION = 1 +_MAX_GOAL_CHARS = 16_000 +_MAX_CONTEXT_CHARS = 32_000 +_MAX_METADATA_BYTES = 8_192 +_MAX_RESULT_CHARS = 32_000 +_TERMINAL_RETENTION_SECONDS = 3_600 + + +class SubagentLifecycleError(ValueError): + """A request cannot be safely accepted by the public lifecycle API.""" + + +class SubagentState(str, enum.Enum): + PENDING = "PENDING" + STARTING = "STARTING" + RUNNING = "RUNNING" + SUCCEEDED = "SUCCEEDED" + FAILED = "FAILED" + INTERRUPTED = "INTERRUPTED" + CANCEL_REQUESTED = "CANCEL_REQUESTED" + CANCELLED = "CANCELLED" + UNKNOWN = "UNKNOWN" + + +@dataclasses.dataclass(frozen=True) +class SubagentLaunchRequest: + goal: str + context: Optional[str] = None + role: str = "leaf" + model: Optional[str] = None + allowed_toolsets: Optional[tuple[str, ...]] = None + blocked_tools: tuple[str, ...] = () + working_directory: Optional[str] = None + parent_session_id: Optional[str] = None + correlation_id: Optional[str] = None + metadata: Mapping[str, Any] = dataclasses.field(default_factory=dict) + timeout_seconds: Optional[float] = None + + +@dataclasses.dataclass(frozen=True) +class SubagentHandle: + contract_version: int + subagent_id: str + parent_session_id: Optional[str] + correlation_id: Optional[str] + created_at: float + provider: Optional[str] + model: Optional[str] + role: str + depth: int + capability: str + + def to_dict(self) -> dict[str, Any]: + return dataclasses.asdict(self) + + @classmethod + def from_dict(cls, value: Mapping[str, Any]) -> "SubagentHandle": + try: + return cls(**dict(value)) + except (TypeError, ValueError) as exc: + raise SubagentLifecycleError("Malformed subagent handle.") from exc + + +@dataclasses.dataclass(frozen=True) +class SubagentStatus: + handle: SubagentHandle + state: SubagentState + updated_at: float + diagnostic: Optional[str] = None + + +@dataclasses.dataclass(frozen=True) +class SubagentTerminalState: + handle: SubagentHandle + state: SubagentState + completed: bool + timed_out: bool = False + diagnostic: Optional[str] = None + + +@dataclasses.dataclass(frozen=True) +class SubagentCancelResult: + accepted: bool + already_terminal: bool = False + unknown_handle: bool = False + unsupported: bool = False + state: SubagentState = SubagentState.UNKNOWN + + +@dataclasses.dataclass(frozen=True) +class SubagentResult: + handle: SubagentHandle + terminal_state: SubagentState + ready: bool + summary: Optional[str] = None + structured_payload: Optional[Mapping[str, Any]] = None + started_at: Optional[float] = None + completed_at: Optional[float] = None + error_classification: Optional[str] = None + error_message: Optional[str] = None + usage_metadata: Mapping[str, Any] = dataclasses.field(default_factory=dict) + tool_execution_summary: Mapping[str, Any] = dataclasses.field(default_factory=dict) + result_hash: Optional[str] = None + + +@dataclasses.dataclass(frozen=True) +class SubagentReconnectResult: + connected: bool + state: SubagentState + diagnostic: Optional[str] = None + + +@dataclasses.dataclass +class _Record: + handle: SubagentHandle + state: SubagentState + updated_at: float + agent: Any = None + future: Optional[Future] = None + started_at: Optional[float] = None + completed_at: Optional[float] = None + result: Optional[SubagentResult] = None + + +class _Registry: + """Thread-safe terminal-retention registry; never returns live records.""" + + def __init__(self) -> None: + self.lock = threading.RLock() + self.records: dict[str, _Record] = {} + self.correlations: dict[tuple[Optional[str], str], str] = {} + + +_REGISTRY = _Registry() +# Daemon worker pool: a wedged/abandoned child must never block interpreter +# exit at atexit-join time (same rationale as _run_single_child's timeout +# executor and the async-delegation registry pool). +from tools.daemon_pool import DaemonThreadPoolExecutor as _DaemonExecutor + +_EXECUTOR = _DaemonExecutor(max_workers=8, thread_name_prefix="hermes-lifecycle") +_SECRET = secrets.token_bytes(32) +_ACTIVE_PARENT_AGENT: contextvars.ContextVar[Any] = contextvars.ContextVar( + "hermes_subagent_lifecycle_parent", default=None +) + + +@contextmanager +def bind_subagent_parent(parent_agent: Any): + """Bind the host-owned parent for the current agent turn.""" + token = _ACTIVE_PARENT_AGENT.set(parent_agent) + try: + yield + finally: + _ACTIVE_PARENT_AGENT.reset(token) + + +def get_active_subagent_parent() -> Any: + """Return the parent bound to this execution context, if any.""" + return _ACTIVE_PARENT_AGENT.get() + + +class SubagentLifecycleService: + """Stable public service returned by :attr:`PluginContext.subagent_lifecycle`. + + Running children are in-process only. Completed results remain available + until process exit; ``reconnect`` accurately reports that a serialized + handle cannot reconnect after a restart instead of launching work again. + """ + + def __init__(self, parent_agent_resolver: Callable[[], Any]) -> None: + self._parent_agent_resolver = parent_agent_resolver + + def launch(self, request: SubagentLaunchRequest) -> SubagentHandle: + parent = self._parent_agent_resolver() + if parent is None: + raise SubagentLifecycleError( + "No active Hermes parent session is available." + ) + self._validate_request(request, parent) + parent_session_id = str(getattr(parent, "session_id", "") or "") or None + if request.parent_session_id and request.parent_session_id != parent_session_id: + raise SubagentLifecycleError( + "parent_session_id does not match the active session." + ) + correlation_key = (parent_session_id, request.correlation_id or "") + with _REGISTRY.lock: + self._cleanup_locked() + if request.correlation_id and correlation_key in _REGISTRY.correlations: + raise SubagentLifecycleError( + "Duplicate correlation_id for this parent session." + ) + + # Delegate construction remains internal so plugin code never imports + # private delegation helpers or manipulates the active-child registry. + from tools.delegate_tool import ( + _build_child_preserving_parent_tools, + DEFAULT_MAX_ITERATIONS, + ) + + child = _build_child_preserving_parent_tools( + task_index=0, + goal=request.goal, + context=request.context, + toolsets=list(request.allowed_toolsets) + if request.allowed_toolsets + else None, + model=request.model, + max_iterations=DEFAULT_MAX_ITERATIONS, + task_count=1, + parent_agent=parent, + role=request.role, + ) + subagent_id = str(getattr(child, "_subagent_id", "") or "") + if not subagent_id: + raise SubagentLifecycleError("Hermes failed to assign a child identity.") + created = time.time() + handle = SubagentHandle( + PUBLIC_CONTRACT_VERSION, + subagent_id, + parent_session_id, + request.correlation_id, + created, + getattr(child, "provider", None), + getattr(child, "model", None), + getattr(child, "_delegate_role", request.role), + int(getattr(child, "_delegate_depth", 1) or 1), + self._capability(subagent_id, parent_session_id, created), + ) + record = _Record(handle, SubagentState.PENDING, created, agent=child) + with _REGISTRY.lock: + _REGISTRY.records[subagent_id] = record + if request.correlation_id: + _REGISTRY.correlations[correlation_key] = subagent_id + record.future = _EXECUTOR.submit(self._run, record, request.goal, parent) + return handle + + def status(self, handle: SubagentHandle) -> SubagentStatus: + record = self._record(handle) + if record is None: + return SubagentStatus( + handle, SubagentState.UNKNOWN, time.time(), "UNKNOWN_HANDLE" + ) + with _REGISTRY.lock: + return SubagentStatus(record.handle, record.state, record.updated_at) + + def wait( + self, handle: SubagentHandle, *, timeout_seconds: Optional[float] = None + ) -> SubagentTerminalState: + record = self._record(handle) + if record is None: + return SubagentTerminalState( + handle, SubagentState.UNKNOWN, True, diagnostic="UNKNOWN_HANDLE" + ) + future = record.future + if future is not None: + try: + future.result(timeout=timeout_seconds) + except TimeoutError: + return SubagentTerminalState(record.handle, record.state, False, True) + except Exception: + pass + with _REGISTRY.lock: + return SubagentTerminalState( + record.handle, record.state, record.result is not None + ) + + def cancel(self, handle: SubagentHandle, *, reason: str) -> SubagentCancelResult: + record = self._record(handle) + if record is None: + return SubagentCancelResult(False, unknown_handle=True) + with _REGISTRY.lock: + if record.result is not None: + return SubagentCancelResult( + False, already_terminal=True, state=record.state + ) + agent = record.agent + record.state = SubagentState.CANCEL_REQUESTED + record.updated_at = time.time() + if agent is None or not hasattr(agent, "interrupt"): + return SubagentCancelResult( + False, unsupported=True, state=SubagentState.CANCEL_REQUESTED + ) + try: + agent.interrupt(f"Lifecycle cancellation requested: {reason[:500]}") + except Exception: + return SubagentCancelResult( + False, unsupported=True, state=SubagentState.CANCEL_REQUESTED + ) + return SubagentCancelResult(True, state=SubagentState.CANCEL_REQUESTED) + + def result(self, handle: SubagentHandle) -> SubagentResult: + record = self._record(handle) + if record is None: + return SubagentResult( + handle, + SubagentState.UNKNOWN, + False, + error_classification="UNKNOWN_HANDLE", + ) + with _REGISTRY.lock: + if record.result is not None: + return record.result + return SubagentResult( + record.handle, record.state, False, error_classification="NOT_READY" + ) + + def reconnect(self, handle: SubagentHandle) -> SubagentReconnectResult: + record = self._record(handle) + if record is None: + return SubagentReconnectResult( + False, SubagentState.UNKNOWN, "RECONNECT_UNAVAILABLE" + ) + with _REGISTRY.lock: + return SubagentReconnectResult(True, record.state) + + def _record(self, handle: SubagentHandle) -> Optional[_Record]: + if ( + not isinstance(handle, SubagentHandle) + or type(handle.contract_version) is not int + or handle.contract_version != PUBLIC_CONTRACT_VERSION + ): + return None + if ( + not isinstance(handle.subagent_id, str) + or not handle.subagent_id + or ( + handle.parent_session_id is not None + and not isinstance(handle.parent_session_id, str) + ) + or ( + handle.correlation_id is not None + and not isinstance(handle.correlation_id, str) + ) + or isinstance(handle.created_at, bool) + or not isinstance(handle.created_at, (int, float)) + or not math.isfinite(handle.created_at) + or (handle.provider is not None and not isinstance(handle.provider, str)) + or (handle.model is not None and not isinstance(handle.model, str)) + or not isinstance(handle.role, str) + or type(handle.depth) is not int + or not isinstance(handle.capability, str) + ): + return None + if not hmac.compare_digest( + handle.capability, + self._capability( + handle.subagent_id, handle.parent_session_id, handle.created_at + ), + ): + return None + parent = self._parent_agent_resolver() + active_parent_id = str(getattr(parent, "session_id", "") or "") or None + if active_parent_id != handle.parent_session_id: + return None + with _REGISTRY.lock: + return _REGISTRY.records.get(handle.subagent_id) + + @staticmethod + def _cleanup_locked() -> None: + """Retain terminal snapshots for a bounded period, never live work.""" + cutoff = time.time() - _TERMINAL_RETENTION_SECONDS + expired = [ + subagent_id + for subagent_id, record in _REGISTRY.records.items() + if record.result is not None + and record.completed_at is not None + and record.completed_at < cutoff + ] + for subagent_id in expired: + record = _REGISTRY.records.pop(subagent_id) + if record.handle.correlation_id: + _REGISTRY.correlations.pop( + (record.handle.parent_session_id, record.handle.correlation_id), + None, + ) + + def _run(self, record: _Record, goal: str, parent: Any) -> None: + with _REGISTRY.lock: + if record.state is not SubagentState.CANCEL_REQUESTED: + record.state = SubagentState.RUNNING + record.started_at = time.time() + record.updated_at = record.started_at + try: + from tools.delegate_tool import _run_child_lifecycle + + raw = _run_child_lifecycle(0, goal, record.agent, parent) + status = ( + str(raw.get("status", "error")) if isinstance(raw, dict) else "error" + ) + if status == "completed": + state = SubagentState.SUCCEEDED + elif status == "interrupted": + state = ( + SubagentState.CANCELLED + if record.state == SubagentState.CANCEL_REQUESTED + else SubagentState.INTERRUPTED + ) + else: + state = SubagentState.FAILED + summary = raw.get("summary") if isinstance(raw, dict) else None + summary = str(summary)[:_MAX_RESULT_CHARS] if summary is not None else None + error = raw.get("error") if isinstance(raw, dict) else None + result = SubagentResult( + record.handle, + state, + True, + summary=summary, + completed_at=time.time(), + started_at=record.started_at, + error_classification=None + if state == SubagentState.SUCCEEDED + else status.upper(), + error_message=str(error)[:_MAX_RESULT_CHARS] if error else None, + usage_metadata={"api_calls": raw.get("api_calls", 0)} + if isinstance(raw, dict) + else {}, + tool_execution_summary={ + "duration_seconds": raw.get("duration_seconds", 0) + } + if isinstance(raw, dict) + else {}, + ) + except Exception as exc: + result = SubagentResult( + record.handle, + SubagentState.FAILED, + True, + started_at=record.started_at, + completed_at=time.time(), + error_classification=type(exc).__name__, + error_message=str(exc)[:_MAX_RESULT_CHARS], + ) + payload = dataclasses.asdict(result) + payload.pop("result_hash", None) + result = dataclasses.replace( + result, + result_hash=hashlib.sha256( + json.dumps(payload, sort_keys=True, default=str).encode() + ).hexdigest(), + ) + with _REGISTRY.lock: + record.agent = None + record.result = result + record.state = result.terminal_state + record.completed_at = result.completed_at + record.updated_at = result.completed_at or time.time() + + @staticmethod + def _capability( + subagent_id: str, parent_session_id: Optional[str], created_at: float + ) -> str: + value = f"{subagent_id}|{parent_session_id or ''}|{created_at:.6f}".encode() + return hmac.new(_SECRET, value, hashlib.sha256).hexdigest() + + @staticmethod + def _validate_request(request: SubagentLaunchRequest, parent: Any) -> None: + if ( + not isinstance(request, SubagentLaunchRequest) + or not isinstance(request.goal, str) + or not request.goal.strip() + or len(request.goal) > _MAX_GOAL_CHARS + ): + raise SubagentLifecycleError( + "goal must be a non-empty string of at most 16000 characters." + ) + if request.context is not None and ( + not isinstance(request.context, str) + or len(request.context) > _MAX_CONTEXT_CHARS + ): + raise SubagentLifecycleError( + "context must be a string of at most 32000 characters." + ) + if request.role not in {"leaf", "orchestrator"}: + raise SubagentLifecycleError("role must be 'leaf' or 'orchestrator'.") + if request.timeout_seconds is not None: + raise SubagentLifecycleError( + "Per-launch timeout is not supported; configure delegation timeout explicitly." + ) + if request.working_directory is not None: + raise SubagentLifecycleError( + "working_directory is not supported because Hermes delegates use isolated task environments." + ) + if request.blocked_tools: + raise SubagentLifecycleError( + "Per-tool blocking is not supported; use allowed_toolsets. Hermes always blocks unsafe child tools." + ) + try: + metadata_bytes = len( + json.dumps(dict(request.metadata), sort_keys=True).encode() + ) + except (TypeError, ValueError) as exc: + raise SubagentLifecycleError("metadata must be JSON-serializable.") from exc + if metadata_bytes > _MAX_METADATA_BYTES: + raise SubagentLifecycleError("metadata exceeds 8192 bytes.") + if request.allowed_toolsets: + from toolsets import TOOLSETS + + unknown = set(request.allowed_toolsets) - set(TOOLSETS) + if unknown: + raise SubagentLifecycleError( + f"Unknown toolsets: {', '.join(sorted(unknown))}." + ) + enabled = getattr(parent, "enabled_toolsets", None) + if enabled is not None and not set(request.allowed_toolsets).issubset( + set(enabled) + ): + raise SubagentLifecycleError( + "Requested toolsets would broaden parent permissions." + ) diff --git a/agent/tool_executor.py b/agent/tool_executor.py index 4815f80db449..7c9fea123296 100644 --- a/agent/tool_executor.py +++ b/agent/tool_executor.py @@ -703,8 +703,15 @@ def execute_tool_calls_concurrent(agent, assistant_message, messages: list, effe _underlying, _underlying_args, _err = _ts.resolve_underlying_call(function_args) if not _err and _underlying: if _underlying in _tool_search_scoped_names(agent): - function_name = _underlying - function_args = _underlying_args + # Probe-validate before unwrapping (ironclaw#5149): + # missing required args return the parameter schema + # instead of dispatching into an opaque failure. + _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. " @@ -1353,8 +1360,25 @@ def execute_tool_calls_sequential(agent, assistant_message, messages: list, effe _underlying, _underlying_args, _err = _ts.resolve_underlying_call(function_args) if not _err and _underlying: if _underlying in _tool_search_scoped_names(agent): - function_name = _underlying - function_args = _underlying_args + # Probe-validate before unwrapping (ironclaw#5149): + # missing required args return the parameter schema + # instead of dispatching into an opaque failure. + _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. " diff --git a/agent/tool_guardrails.py b/agent/tool_guardrails.py index f08f1b604786..444ce3739596 100644 --- a/agent/tool_guardrails.py +++ b/agent/tool_guardrails.py @@ -79,6 +79,7 @@ class ToolCallGuardrailConfig: no_progress_block_after: int = 5 idempotent_tools: frozenset[str] = field(default_factory=lambda: IDEMPOTENT_TOOL_NAMES) mutating_tools: frozenset[str] = field(default_factory=lambda: MUTATING_TOOL_NAMES) + loop_caps: "LoopCapConfig" = field(default_factory=lambda: LoopCapConfig()) @classmethod def from_mapping(cls, data: Mapping[str, Any] | None) -> "ToolCallGuardrailConfig": @@ -121,6 +122,54 @@ class ToolCallGuardrailConfig: hard_stop_after.get("idempotent_no_progress", data.get("no_progress_block_after")), defaults.no_progress_block_after, ), + loop_caps=LoopCapConfig.from_mapping(data.get("loop_caps")), + ) + + +# Default session-wide caps, matching Claude Code's v2.1.212 runaway-loop +# Per-turn (per-agent-loop) caps on runaway-prone tool calls. Counts reset at +# the start of every agent loop (reset_for_turn), so the limit is "within a +# single turn" rather than cumulative over the whole session. A single loop +# issuing dozens of web searches or spawning dozens of subagents is already +# pathological, so the defaults are deliberately low. +_DEFAULT_MAX_WEB_SEARCHES_PER_TURN = 50 +_DEFAULT_MAX_SUBAGENTS_PER_TURN = 50 + + +@dataclass(frozen=True) +class LoopCapConfig: + """Per-turn caps on runaway-prone tool calls. + + Inspired by Claude Code v2.1.212 (Week 29, July 2026), which added caps on + WebSearch calls and subagent spawns to stop runaway search / delegation + loops. Here the caps count *within a single agent loop* (one turn): the + counters reset in ``reset_for_turn`` at the start of every + ``run_conversation``, so a legitimate multi-turn session is never starved, + but a single turn that spirals into an unbounded search / delegation loop + is stopped. + + Semantics differ from the per-turn loop *detector* above (which keys on + repeated identical/failing calls): these caps are a hard ceiling on the + total count of a tool within the turn and fire regardless of + ``hard_stop_enabled``. A value of ``0`` disables the cap (unlimited). + """ + + max_web_searches: int = _DEFAULT_MAX_WEB_SEARCHES_PER_TURN + max_subagents: int = _DEFAULT_MAX_SUBAGENTS_PER_TURN + + @classmethod + def from_mapping(cls, data: Mapping[str, Any] | None) -> "LoopCapConfig": + """Build config from the ``tool_loop_guardrails.loop_caps`` section.""" + if not isinstance(data, Mapping): + return cls() + defaults = cls() + return cls( + max_web_searches=_non_negative_int( + data.get("max_web_searches"), defaults.max_web_searches + ), + max_subagents=_non_negative_int( + data.get("max_subagents"), defaults.max_subagents + ), ) @@ -233,6 +282,11 @@ class ToolCallGuardrailController: self._same_tool_failure_counts: dict[str, int] = {} self._no_progress: dict[ToolCallSignature, tuple[str, int]] = {} self._halt_decision: ToolGuardrailDecision | None = None + # Per-turn runaway-loop cap counters. Reset every turn (this method + # runs at the start of each run_conversation), so the caps bound a + # single agent loop rather than accumulating across the session. + self._turn_web_search_count = 0 + self._turn_subagent_count = 0 @property def halt_decision(self) -> ToolGuardrailDecision | None: @@ -240,6 +294,17 @@ class ToolCallGuardrailController: def before_call(self, tool_name: str, args: Mapping[str, Any] | None) -> ToolGuardrailDecision: signature = ToolCallSignature.from_call(tool_name, _coerce_args(args)) + + # ── Per-turn runaway-loop caps ────────────────────────────────── + # These are hard ceilings on how many times a runaway-prone tool may + # be called within a single agent loop (turn). They apply regardless + # of hard_stop_enabled (which only governs the per-turn loop detector). + # We block BEFORE the call runs once the count is already at the cap, + # then increment for an allowed call so the (cap+1)-th is refused. + cap_block = self._check_loop_cap(tool_name, _coerce_args(args), signature) + if cap_block is not None: + return cap_block + if not self.config.hard_stop_enabled: return ToolGuardrailDecision(tool_name=tool_name, signature=signature) @@ -379,6 +444,68 @@ class ToolCallGuardrailController: return False return tool_name in self.config.idempotent_tools + def _check_loop_cap( + self, + tool_name: str, + args: Mapping[str, Any], + signature: ToolCallSignature, + ) -> ToolGuardrailDecision | None: + """Enforce and advance the per-turn runaway-loop counters. + + Returns a ``block`` decision when the cap is already reached, otherwise + increments the relevant counter for the allowed call and returns + ``None``. A cap of 0 disables that limit entirely. Counters reset each + turn via ``reset_for_turn``. + """ + caps = self.config.loop_caps + + if tool_name == "web_search": + cap = caps.max_web_searches + if cap and self._turn_web_search_count >= cap: + decision = ToolGuardrailDecision( + action="block", + code="loop_web_search_cap", + message=( + f"Blocked web_search: this turn has already made {cap} " + "web searches, the per-turn limit. This looks like a " + "runaway search loop. Work with the results you already " + "have and give the user your answer." + ), + tool_name=tool_name, + count=self._turn_web_search_count, + signature=signature, + ) + self._halt_decision = decision + return decision + self._turn_web_search_count += 1 + return None + + if tool_name == "delegate_task": + cap = caps.max_subagents + if not cap: + return None + spawn_count = _subagent_spawn_count(args) + if self._turn_subagent_count >= cap: + decision = ToolGuardrailDecision( + action="block", + code="loop_subagent_cap", + message=( + f"Blocked delegate_task: this turn has already spawned " + f"{self._turn_subagent_count} subagents (limit {cap}). " + "This looks like a runaway delegation loop. Finish the " + "work with the results you have and answer the user." + ), + tool_name=tool_name, + count=self._turn_subagent_count, + signature=signature, + ) + self._halt_decision = decision + return decision + self._turn_subagent_count += spawn_count + return None + + return None + def toolguard_synthetic_result(decision: ToolGuardrailDecision) -> str: """Build a synthetic role=tool content string for a blocked tool call.""" @@ -471,6 +598,32 @@ def _positive_int(value: Any, default: int) -> int: return parsed if parsed >= 1 else default +def _non_negative_int(value: Any, default: int) -> int: + """Parse a session-cap value. 0 is a valid (disable) value; negatives and + junk fall back to the default.""" + if value is None: + return default + try: + parsed = int(value) + except (TypeError, ValueError): + return default + return parsed if parsed >= 0 else default + + +def _subagent_spawn_count(args: Mapping[str, Any]) -> int: + """How many subagents a single delegate_task call spawns. + + delegate_task runs in one of two modes: a batch (``tasks`` is a non-empty + list, one child per item) or a single task (``goal``). Count the batch size + when present, otherwise 1, so the session subagent cap reflects real spawns + rather than delegate_task invocations. + """ + tasks = args.get("tasks") if isinstance(args, Mapping) else None + if isinstance(tasks, list) and tasks: + return len(tasks) + return 1 + + def _sha256(value: str) -> str: # surrogatepass: tool results scraped from the web can carry unpaired # UTF-16 surrogates (e.g. half of a mathematical-bold pair); a strict diff --git a/apps/desktop/e2e/large-session-resume.spec.ts b/apps/desktop/e2e/large-session-resume.spec.ts index b39ab3f1a4d7..02c5f16937d1 100644 --- a/apps/desktop/e2e/large-session-resume.spec.ts +++ b/apps/desktop/e2e/large-session-resume.spec.ts @@ -19,6 +19,12 @@ import { RealSessionBuilder } from './real-session-builder' const DESKTOP_ROOT = path.resolve(import.meta.dirname, '..') const SESSION_TITLE = 'E2E large persisted session' const EXPECTED_TEXT = 'E2E persisted user message 52' +// The oldest seeded turn (HISTORY_TURNS[0]). The transcript first paints only +// the newest turns (FIRST_PAINT_BUDGET) and backfills the rest in a rAF; a +// baseline count taken before that backfill sees a clipped transcript and +// falsely reports duplicates once the full list mounts. Waiting for this +// oldest row means the baseline reflects the fully-mounted transcript. +const OLDEST_SEEDED_TEXT = 'E2E persisted user message 0: audit the compatibility matrix' const BACKGROUND_PROMPT = 'E2E background inference must remain attached across resume' const HISTORY_TURNS = Array.from( { length: 27 }, @@ -210,6 +216,16 @@ test.describe('large session resume', () => { await waitForAppReady(fixture, 120_000) await openSeededSession(fixture.page) + // The transcript first paints only the newest turns (FIRST_PAINT_BUDGET) + // and backfills older turns in a rAF. Wait for the oldest seeded row to + // mount before taking the baseline so it reflects the full transcript — + // otherwise a clipped baseline makes the backfilled rows look like + // duplicates of the completed reply. + await fixture.page.waitForFunction( + expected => (document.querySelector('[data-slot="aui_thread-viewport"]')?.textContent ?? '').includes(expected), + OLDEST_SEEDED_TEXT, + { timeout: 30_000 }, + ) const initialMockReplyCount = await textNodeOccurrences(fixture.page, MOCK_REPLY) await submitPrompt(fixture.page, BACKGROUND_PROMPT) await fixture.mock.waitForHeldStream() diff --git a/apps/desktop/e2e/session-compression-and-queue-stop.spec.ts b/apps/desktop/e2e/session-compression-and-queue-stop.spec.ts index 911f15c78407..e48c52cda2b0 100644 --- a/apps/desktop/e2e/session-compression-and-queue-stop.spec.ts +++ b/apps/desktop/e2e/session-compression-and-queue-stop.spec.ts @@ -4,11 +4,7 @@ import { expect, test, type Page } from '@playwright/test' -import { - type MockBackendFixture, - setupMockBackend, - waitForAppReady, -} from './fixtures' +import { type MockBackendFixture, setupMockBackend, waitForAppReady } from './fixtures' import { MOCK_REPLY, receivedUserTexts, restartMockServer } from './mock-server' async function send(page: Page, text: string, delay = 15): Promise { @@ -25,12 +21,11 @@ async function pasteAndSend(page: Page, text: string): Promise { await page.keyboard.press('Enter') } - async function waitForTranscript(page: Page, text: string, timeout = 90_000): Promise { await page.waitForFunction( expected => document.querySelector('[data-slot="aui_thread-viewport"]')?.textContent?.includes(expected) ?? false, text, - { timeout }, + { timeout } ) } @@ -62,23 +57,16 @@ test.describe('session compression', () => { await send(page, 'E2E_COMPRESSION_THIRD') await expect.poll(() => receivedUserTexts().filter(text => text === 'E2E_COMPRESSION_THIRD').length).toBe(1) - // Commit the command before typing its argument. This waits for the async - // completion request on cold CI workers, then uses the composer's own - // keyboard accept path to replace the `/compress` trigger with a command - // chip. Clicking a later completion after typing the argument can insert a - // second command token (for example `//compress ...`) as plain text. + // This test covers compression and continuation, not slash completion. + // Insert the complete command atomically and click Send so an async + // completion response cannot consume Enter as a picker acceptance. const composer = page.locator('[contenteditable="true"]').first() await composer.click() - await composer.type('/compress', { delay: 15 }) - await page.getByText('/compress').first().waitFor({ state: 'visible' }) - await page.keyboard.press('Enter') - await composer.type(' preserve the three test turns', { delay: 15 }) - await page.keyboard.press('Enter') + await page.keyboard.insertText('/compress preserve the three test turns') + await expect.poll(() => composer.textContent()).toContain('preserve the three test turns') + await page.getByRole('button', { name: 'Send', exact: true }).click() await expect - .poll( - () => page.locator('[data-slot="aui_thread-viewport"]').textContent(), - { timeout: 90_000 }, - ) + .poll(() => page.locator('[data-slot="aui_thread-viewport"]').textContent(), { timeout: 90_000 }) .toMatch(/Compressed|No changes from compression/) // Compression rotates the agent's live session id. A post-compression @@ -105,7 +93,7 @@ auxiliary: provider: custom model: mock-model`, mockServer: { - holdFirstCompletionContaining: 'You are a summarization agent creating a context checkpoint.', + holdFirstCompletionContaining: 'You are a summarization agent creating a context checkpoint.' } }) await waitForAppReady(fixture, 120_000) diff --git a/apps/desktop/scripts/diag-code-live.mjs b/apps/desktop/scripts/diag-code-live.mjs new file mode 100644 index 000000000000..98ec39973d9a --- /dev/null +++ b/apps/desktop/scripts/diag-code-live.mjs @@ -0,0 +1,25 @@ +// Is the tree-split preview path actually active in the running renderer? +// Checks the served source (what vite compiled) rather than guessing. +import { attach } from './perf/lib/launch.mjs' + +const { cdp, teardown } = await attach({ port: 9222 }) + +try { + await cdp.send('Runtime.enable') + + const out = await cdp.eval(`(async () => { + const res = await fetch('/src/components/pane-shell/tree/renderer/tree-split.tsx') + const src = await res.text() + return JSON.stringify({ + previewShift: src.includes('previewShift'), + adaptiveFloor: (await (await fetch('/src/app/session/hooks/use-message-stream/index.ts')).text()).includes('adaptiveFloor'), + structuralSignature: (await (await fetch('/src/components/assistant-ui/thread/list.tsx')).text()).includes('structuralSignature'), + sharedRO: (await (await fetch('/src/hooks/use-resize-observer.ts')).text()).includes('sharedObserver'), + rootTipProvider: (await (await fetch('/src/main.tsx')).text()).includes('RootTooltipProvider') + }) + })()`) + + console.log(out) +} finally { + teardown?.() +} diff --git a/apps/desktop/scripts/diag-key-latency.mjs b/apps/desktop/scripts/diag-key-latency.mjs new file mode 100644 index 000000000000..7a29bd46cad0 --- /dev/null +++ b/apps/desktop/scripts/diag-key-latency.mjs @@ -0,0 +1,47 @@ +// Typing latency, isolated: keystroke -> next paint, with and without an +// active stream. Distinguishes "input is slow" from "the frame budget is +// consumed by streaming flushes" — the fix differs completely. +import { attach } from './perf/lib/launch.mjs' + +const { cdp, teardown } = await attach({ port: 9222 }) + +const TYPE = ` + (async () => { + const el = [...document.querySelectorAll('[contenteditable="true"]')].find(e => e.offsetParent) + if (!el) return JSON.stringify({ error: 'no composer' }) + el.focus() + const perKey = [] + for (let i = 0; i < 30; i++) { + const ch = 'abcdefghij'[i % 10] + const t0 = performance.now() + el.dispatchEvent(new KeyboardEvent('keydown', { bubbles: true, key: ch })) + el.textContent += ch + el.dispatchEvent(new InputEvent('input', { bubbles: true, data: ch, inputType: 'insertText' })) + el.dispatchEvent(new KeyboardEvent('keyup', { bubbles: true, key: ch })) + await new Promise(r => requestAnimationFrame(r)) + perKey.push(performance.now() - t0) + // Human-ish 80ms cadence so streaming flushes interleave realistically. + await new Promise(r => setTimeout(r, 80)) + } + el.textContent = '' + el.dispatchEvent(new InputEvent('input', { bubbles: true, inputType: 'deleteContentBackward' })) + const sorted = [...perKey].sort((a, b) => a - b) + const pct = p => sorted[Math.min(sorted.length - 1, Math.floor(sorted.length * p))] + const busy = (() => { try { return document.querySelectorAll('[data-status="running"]').length } catch { return -1 } })() + return JSON.stringify({ + keyToPaint_p50: Math.round(pct(0.5) * 10) / 10, + keyToPaint_p95: Math.round(pct(0.95) * 10) / 10, + worst: Math.round(sorted[sorted.length - 1] * 10) / 10, + over16: perKey.filter(f => f > 16.7).length, + over33: perKey.filter(f => f > 33).length, + streamingParts: busy + }) + })() +` + +try { + await cdp.send('Runtime.enable') + console.log(await cdp.eval(TYPE)) +} finally { + teardown?.() +} diff --git a/apps/desktop/scripts/diag-live-state.mjs b/apps/desktop/scripts/diag-live-state.mjs new file mode 100644 index 000000000000..7526bf0e65fb --- /dev/null +++ b/apps/desktop/scripts/diag-live-state.mjs @@ -0,0 +1,21 @@ +// Quick state probe of the running hgui instance via CDP. +import { attach } from './perf/lib/launch.mjs' + +const { cdp, teardown } = await attach({ port: 9222 }) + +try { + await cdp.send('Runtime.enable') + + const state = await cdp.eval(`(() => { + const rc = !!window.__RENDER_COUNTS__ + const pl = !!window.__PERF_LIVE__ + const tiles = window.__HERMES_SESSION_TILES__ ? Object.keys(window.__HERMES_SESSION_TILES__.states()).length : -1 + const gw = document.querySelector('[data-slot="statusbar"]')?.textContent?.slice(0, 120) ?? '(no statusbar)' + const sidebarRows = document.querySelectorAll('[data-slot="sidebar"] [data-session-id], [data-tree-group] a').length + return JSON.stringify({ rc, pl, tiles, gw, sidebarRows, title: document.title, url: location.href.slice(0, 80) }) + })()`) + + console.log(state) +} finally { + teardown?.() +} diff --git a/apps/desktop/scripts/diag-real-loop.mjs b/apps/desktop/scripts/diag-real-loop.mjs new file mode 100644 index 000000000000..e3bde3b38088 --- /dev/null +++ b/apps/desktop/scripts/diag-real-loop.mjs @@ -0,0 +1,149 @@ +// The real-app perf loop: drive HER hgui instance (real profile, real +// sessions) through the three interactions that matter — session switch, +// sidebar drag, composer typing — and report honest single-clock numbers. +// +// node scripts/diag-real-loop.mjs [--port 9222] [--switches 6] +// +// Unlike the synthetic scenarios this clicks REAL sidebar rows, so session +// switching is measured as the user feels it: click -> transcript painted. + +import { attach } from './perf/lib/launch.mjs' +import { sleep } from './perf/lib/cdp.mjs' + +const arg = (name, fallback) => { + const i = process.argv.indexOf(`--${name}`) + + return i === -1 ? fallback : process.argv[i + 1] +} + +const port = Number(arg('port', 9222)) +const SWITCHES = Number(arg('switches', 6)) + +const { cdp, teardown } = await attach({ port }) + +// --------------------------------------------------------------------------- +// Session switch: click a sidebar session row, await the transcript settling. +// Measures click -> first paint of the new transcript AND click -> settled +// (two rAFs with no further DOM mutation in the thread viewport). +// --------------------------------------------------------------------------- +const SWITCH = swaps => ` + (async () => { + const rows = [...document.querySelectorAll('[data-slot="row-button"]')] + .filter(el => el.offsetParent && (el.textContent ?? '').trim()) + if (rows.length < 2) return JSON.stringify({ error: 'need 2+ visible session rows, found ' + rows.length }) + + const results = [] + for (let i = 0; i < ${swaps}; i++) { + const row = rows[i % Math.min(rows.length, 4)] + const viewport = () => document.querySelector('[data-slot="aui_thread-viewport"]') + const t0 = performance.now() + row.dispatchEvent(new PointerEvent('pointerdown', { bubbles: true, cancelable: true, pointerId: 1, isPrimary: true, button: 0, buttons: 1 })) + row.dispatchEvent(new PointerEvent('pointerup', { bubbles: true, cancelable: true, pointerId: 1, isPrimary: true, button: 0 })) + row.click() + + // First paint: next two rAFs after the click. + await new Promise(r => requestAnimationFrame(() => requestAnimationFrame(r))) + const firstPaint = performance.now() - t0 + + // Settled: no mutations in the viewport for 2 consecutive frames, capped 3s. + let lastMutation = performance.now() + const target = viewport() ?? document.body + const mo = new MutationObserver(() => { lastMutation = performance.now() }) + mo.observe(target, { childList: true, subtree: true, characterData: true }) + const deadline = performance.now() + 3000 + while (performance.now() < deadline) { + await new Promise(r => requestAnimationFrame(r)) + if (performance.now() - lastMutation > 120) break + } + mo.disconnect() + results.push({ firstPaint: Math.round(firstPaint), settled: Math.round(performance.now() - t0 - 120) }) + await new Promise(r => setTimeout(r, 250)) + } + return JSON.stringify(results) + })() +` + +// --------------------------------------------------------------------------- +// Drag the first visible sash, single-clock frames. +// --------------------------------------------------------------------------- +const DRAG = ` + (async () => { + const handle = [...document.querySelectorAll('[role="separator"]')].find(el => el.offsetParent || el.getBoundingClientRect().width > 0) + if (!handle) return JSON.stringify({ error: 'no sash' }) + const box = handle.getBoundingClientRect() + const y = box.top + box.height / 2 + const x0 = box.left + box.width / 2 + let x = x0 + const o = { bubbles: true, cancelable: true, pointerId: 1, pointerType: 'mouse', isPrimary: true, button: 0, buttons: 1 } + const frames = [] + let last = performance.now() + handle.dispatchEvent(new PointerEvent('pointerdown', { ...o, clientX: x, clientY: y })) + for (let i = 0; i < 60; i++) { + x += (i < 30 ? 2 : -2) + window.dispatchEvent(new PointerEvent('pointermove', { ...o, clientX: x, clientY: y })) + await new Promise(r => requestAnimationFrame(r)) + const now = performance.now(); frames.push(now - last); last = now + } + window.dispatchEvent(new PointerEvent('pointerup', { ...o, buttons: 0, clientX: x, clientY: y })) + const total = frames.reduce((a, b) => a + b, 0) + const sorted = [...frames].sort((a, b) => a - b) + return JSON.stringify({ + fps: Math.round((frames.length / total) * 1000 * 10) / 10, + p95: Math.round(sorted[Math.floor(sorted.length * 0.95)] * 10) / 10, + worst: Math.round(sorted[sorted.length - 1] * 10) / 10, + slow33: frames.filter(f => f > 33).length + }) + })() +` + +// --------------------------------------------------------------------------- +// Type into the composer, single-clock frames (one mark per keystroke frame). +// --------------------------------------------------------------------------- +const TYPE = ` + (async () => { + const el = [...document.querySelectorAll('[contenteditable="true"]')].find(e => e.offsetParent) + if (!el) return JSON.stringify({ error: 'no composer' }) + el.focus() + const frames = [] + let last = performance.now() + for (let i = 0; i < 40; i++) { + const ch = 'the quick brown fox '[i % 20] + el.dispatchEvent(new KeyboardEvent('keydown', { bubbles: true, key: ch })) + el.textContent += ch + el.dispatchEvent(new InputEvent('input', { bubbles: true, data: ch, inputType: 'insertText' })) + el.dispatchEvent(new KeyboardEvent('keyup', { bubbles: true, key: ch })) + await new Promise(r => requestAnimationFrame(r)) + const now = performance.now(); frames.push(now - last); last = now + await new Promise(r => setTimeout(r, 20)) + } + // Clear what we typed. + el.textContent = '' + el.dispatchEvent(new InputEvent('input', { bubbles: true, inputType: 'deleteContentBackward' })) + const moving = frames + const total = moving.reduce((a, b) => a + b, 0) + const sorted = [...moving].sort((a, b) => a - b) + return JSON.stringify({ + fps: Math.round((moving.length / total) * 1000 * 10) / 10, + p95: Math.round(sorted[Math.floor(sorted.length * 0.95)] * 10) / 10, + worst: Math.round(sorted[sorted.length - 1] * 10) / 10, + slow33: moving.filter(f => f > 33).length + }) + })() +` + +try { + await cdp.send('Runtime.enable') + + console.log('== SESSION SWITCH (click -> paint / settled ms) ==') + console.log(await cdp.eval(SWITCH(SWITCHES))) + + await sleep(500) + console.log('\n== SIDEBAR DRAG ==') + console.log(await cdp.eval(DRAG)) + + await sleep(500) + console.log('\n== COMPOSER TYPING ==') + console.log(await cdp.eval(TYPE)) +} finally { + teardown?.() +} diff --git a/apps/desktop/scripts/diag-sidebar-dom.mjs b/apps/desktop/scripts/diag-sidebar-dom.mjs new file mode 100644 index 000000000000..64159ca6d5e0 --- /dev/null +++ b/apps/desktop/scripts/diag-sidebar-dom.mjs @@ -0,0 +1,27 @@ +// Dump the sidebar's actual DOM shape so selectors stop being guesses. +import { attach } from './perf/lib/launch.mjs' + +const { cdp, teardown } = await attach({ port: 9222 }) + +try { + await cdp.send('Runtime.enable') + + const out = await cdp.eval(`(() => { + const sidebar = document.querySelector('[data-slot="sidebar"]') ?? document.querySelector('aside') + if (!sidebar) return '(no sidebar el)' + // Find clickable rows: anchors or buttons with text, depth-limited sample. + const clickables = [...sidebar.querySelectorAll('a, button, [role="button"], [data-slot]')].slice(0, 60) + const rows = clickables.map(el => ({ + tag: el.tagName.toLowerCase(), + slot: el.getAttribute('data-slot') ?? '', + cls: (el.className?.baseVal ?? el.className ?? '').toString().slice(0, 40), + text: (el.textContent ?? '').trim().slice(0, 30), + visible: !!el.offsetParent + })).filter(r => r.text) + return JSON.stringify(rows.slice(0, 30), null, 1) + })()`) + + console.log(out) +} finally { + teardown?.() +} diff --git a/apps/desktop/scripts/diag-switch-autopsy.mjs b/apps/desktop/scripts/diag-switch-autopsy.mjs new file mode 100644 index 000000000000..f90e0d529824 --- /dev/null +++ b/apps/desktop/scripts/diag-switch-autopsy.mjs @@ -0,0 +1,52 @@ +// Session-switch autopsy: click between the two heaviest rows repeatedly, +// recording per-switch (a) settled ms, (b) React commits, (c) top rendered +// components — so slow switches name themselves. +import { attach } from './perf/lib/launch.mjs' +import { sleep } from './perf/lib/cdp.mjs' + +const arg = (name, fallback) => { + const i = process.argv.indexOf(`--${name}`) + + return i === -1 ? fallback : process.argv[i + 1] +} + +const port = Number(arg('port', 9222)) +const ROUNDS = Number(arg('rounds', 8)) + +const { cdp, teardown } = await attach({ port }) + +const SWITCH_ONE = index => ` + (async () => { + const rows = [...document.querySelectorAll('[data-slot="row-button"]')].filter(el => el.offsetParent) + if (rows.length < 2) return JSON.stringify({ error: 'rows' }) + const row = rows[${index} % 2] + const rc = window.__RENDER_COUNTS__ + rc.start() + const t0 = performance.now() + row.click() + let lastMutation = performance.now() + const mo = new MutationObserver(() => { lastMutation = performance.now() }) + mo.observe(document.body, { childList: true, subtree: true, characterData: true }) + const deadline = performance.now() + 4000 + while (performance.now() < deadline) { + await new Promise(r => requestAnimationFrame(r)) + if (performance.now() - lastMutation > 150) break + } + mo.disconnect() + rc.stop() + const settled = Math.round(performance.now() - t0 - 150) + const report = rc.report(6).map(r => r.name + ':' + r.renders + '(' + Math.round(r.totalMs) + 'ms)') + return JSON.stringify({ label: (row.textContent ?? '').slice(0, 24), settled, commits: rc.commits(), top: report }) + })() +` + +try { + await cdp.send('Runtime.enable') + + for (let i = 0; i < ROUNDS; i++) { + console.log(await cdp.eval(SWITCH_ONE(i))) + await sleep(400) + } +} finally { + teardown?.() +} diff --git a/apps/desktop/scripts/diag-switch-trace.mjs b/apps/desktop/scripts/diag-switch-trace.mjs new file mode 100644 index 000000000000..8efe5462e3fe --- /dev/null +++ b/apps/desktop/scripts/diag-switch-trace.mjs @@ -0,0 +1,74 @@ +// What happens during a SLOW session switch? Click a heavy row with tracing +// on, dump the style/layout/script split plus top callsites. +import { attach } from './perf/lib/launch.mjs' +import { sleep } from './perf/lib/cdp.mjs' + +const { cdp, teardown } = await attach({ port: 9222 }) + +const CLICK_HEAVIEST = ` + (() => { + const rows = [...document.querySelectorAll('[data-slot="row-button"]')].filter(el => el.offsetParent) + if (rows.length < 2) return 'need rows' + // Alternate between the first two rows so every run actually switches. + const current = location.hash + const target = rows.find(r => !r.getAttribute('data-active')) ?? rows[1] + target.click() + return 'clicked: ' + (target.textContent ?? '').slice(0, 40) + })() +` + +const events = [] +let complete = false +cdp.on('Tracing.dataCollected', p => events.push(...(p.value ?? []))) +cdp.on('Tracing.tracingComplete', () => { + complete = true +}) + +try { + await cdp.send('Runtime.enable') + + await cdp.send('Tracing.start', { + transferMode: 'ReportEvents', + traceConfig: { includedCategories: ['devtools.timeline'] } + }) + + console.log(await cdp.eval(CLICK_HEAVIEST)) + await sleep(2500) + console.log(await cdp.eval(CLICK_HEAVIEST)) + await sleep(2500) + + await cdp.send('Tracing.end') + + for (let w = 0; !complete && w < 10000; w += 200) { + await sleep(200) + } + + const totals = new Map() + const byFn = new Map() + + for (const e of events) { + if (e.ph !== 'X' || typeof e.dur !== 'number') { + continue + } + + totals.set(e.name, (totals.get(e.name) ?? 0) + e.dur / 1000) + + if (e.name === 'FunctionCall') { + const d = e.args?.data ?? {} + const key = `${d.functionName || '(anon)'} @ ${(d.url || '?').split('/').pop()}:${d.lineNumber ?? '?'}` + byFn.set(key, (byFn.get(key) ?? 0) + e.dur / 1000) + } + } + + const style = totals.get('UpdateLayoutTree') ?? 0 + const layout = totals.get('Layout') ?? 0 + const script = (totals.get('FunctionCall') ?? 0) + (totals.get('EvaluateScript') ?? 0) + (totals.get('TimerFire') ?? 0) + console.log(`\nVERDICT over 2 switches: style=${style.toFixed(0)}ms layout=${layout.toFixed(0)}ms script=${script.toFixed(0)}ms paint=${(totals.get('Paint') ?? 0).toFixed(0)}ms`) + console.log('\nTOP CALLSITES:') + + for (const [name, ms] of [...byFn.entries()].sort((a, b) => b[1] - a[1]).slice(0, 12)) { + console.log(` ${ms.toFixed(1).padStart(8)} ${name}`) + } +} finally { + teardown?.() +} diff --git a/apps/desktop/scripts/live-drive.mjs b/apps/desktop/scripts/live-drive.mjs new file mode 100644 index 000000000000..a20d09f30cdd --- /dev/null +++ b/apps/desktop/scripts/live-drive.mjs @@ -0,0 +1,290 @@ +// Live-drive harness for the REAL hgui instance on :9222. +// +// node scripts/live-drive.mjs status — targets, session count, perf-live armed? +// node scripts/live-drive.mjs fps [seconds] — raw rAF fps over N seconds (default 4) +// node scripts/live-drive.mjs drag — drag the sidebar sash, report fps + LoAF +// node scripts/live-drive.mjs type — type into composer, report fps + LoAF +// node scripts/live-drive.mjs switch — cycle through sidebar sessions, per-switch ms +// node scripts/live-drive.mjs send "msg" — submit a prompt in the focused session +// node scripts/live-drive.mjs eval "expr" — arbitrary page eval +// +// Attaches to the page target directly (no perf-harness deps) so it works on +// the app Brooklyn actually runs, with her profile, her sessions, her layout. + +import { WebSocket } from 'ws' + +const PORT = Number(process.env.CDP_PORT ?? 9222) + +async function attach() { + const list = await (await fetch(`http://127.0.0.1:${PORT}/json/list`)).json() + const page = list.find(t => t.type === 'page' && !/devtools/.test(t.url)) + + if (!page) { + throw new Error('no page target on :' + PORT) + } + + const ws = new WebSocket(page.webSocketDebuggerUrl, { maxPayload: 256 * 1024 * 1024 }) + await new Promise((resolve, reject) => { + ws.once('open', resolve) + ws.once('error', reject) + }) + + let id = 0 + const pending = new Map() + ws.on('message', raw => { + const msg = JSON.parse(raw) + + if (msg.id && pending.has(msg.id)) { + const { resolve, reject } = pending.get(msg.id) + pending.delete(msg.id) + msg.error ? reject(new Error(msg.error.message)) : resolve(msg.result) + } + }) + + const send = (method, params = {}) => + new Promise((resolve, reject) => { + const mid = ++id + pending.set(mid, { resolve, reject }) + ws.send(JSON.stringify({ id: mid, method, params })) + }) + + await send('Runtime.enable') + + const evaluate = async expression => { + const r = await send('Runtime.evaluate', { expression, returnByValue: true, awaitPromise: true }) + + if (r.exceptionDetails) { + throw new Error(r.exceptionDetails.exception?.description ?? 'eval failed') + } + + return r.result?.value + } + + return { evaluate, close: () => ws.close(), send } +} + +const FPS = seconds => ` + (async () => { + const frames = [] + let last = performance.now() + const end = last + ${seconds * 1000} + while (performance.now() < end) { + await new Promise(r => requestAnimationFrame(r)) + const now = performance.now() + frames.push(now - last) + last = now + } + const total = frames.reduce((a, b) => a + b, 0) + const sorted = [...frames].sort((a, b) => a - b) + const pct = p => sorted[Math.min(sorted.length - 1, Math.floor(sorted.length * p))] + return { + fps: Math.round((frames.length / total) * 1000 * 10) / 10, + p95: Math.round(pct(0.95) * 10) / 10, + worst: Math.round(sorted[sorted.length - 1] * 10) / 10, + slow33: frames.filter(f => f > 33).length, + n: frames.length + } + })() +` + +// LoAF recorder for a window of work driven inside `body`. +const WITH_LOAF = body => ` + (async () => { + const lofs = [] + const po = new PerformanceObserver(list => { + for (const e of list.getEntries()) { + lofs.push({ + ms: Math.round(e.duration), + block: Math.round(e.blockingDuration ?? 0), + style: e.styleAndLayoutStart ? Math.round(e.startTime + e.duration - e.styleAndLayoutStart) : 0, + scripts: (e.scripts ?? []).filter(s => s.duration >= 5).map(s => + (s.invokerType ?? '') + ':' + (s.invoker ?? s.sourceFunctionName ?? '?') + '@' + + ((s.sourceURL ?? '').split('/').pop() ?? '') + ' ' + Math.round(s.duration) + 'ms') + }) + } + }) + po.observe({ type: 'long-animation-frame', buffered: false }) + const frames = [] + let last = performance.now() + let stop = false + const tick = () => { + if (stop) return + const now = performance.now() + frames.push(now - last) + last = now + requestAnimationFrame(tick) + } + requestAnimationFrame(tick) + ${body} + stop = true + po.disconnect() + const total = frames.reduce((a, b) => a + b, 0) + const sorted = [...frames].sort((a, b) => a - b) + const pct = p => sorted.length ? sorted[Math.min(sorted.length - 1, Math.floor(sorted.length * p))] : 0 + return { + fps: total ? Math.round((frames.length / total) * 1000 * 10) / 10 : 0, + p95: Math.round(pct(0.95) * 10) / 10, + worst: sorted.length ? Math.round(sorted[sorted.length - 1] * 10) / 10 : 0, + slow33: frames.filter(f => f > 33).length, + longFrames: lofs.slice(0, 10) + } + })() +` + +const DRAG_BODY = ` + const handle = document.querySelector('[role="separator"]') + if (!handle) throw new Error('no sash') + const box = handle.getBoundingClientRect() + const y = box.top + box.height / 2 + const x0 = box.left + box.width / 2 + let x = x0 + const opts = { bubbles: true, cancelable: true, pointerId: 1, pointerType: 'mouse', isPrimary: true, button: 0, buttons: 1 } + handle.dispatchEvent(new PointerEvent('pointerdown', { ...opts, clientX: x, clientY: y })) + for (let i = 0; i < 60; i++) { + x += (i < 30 ? 3 : -3) + window.dispatchEvent(new PointerEvent('pointermove', { ...opts, clientX: x, clientY: y })) + await new Promise(r => requestAnimationFrame(r)) + } + window.dispatchEvent(new PointerEvent('pointerup', { ...opts, buttons: 0, clientX: x, clientY: y })) +` + +const TYPE_BODY = ` + const el = [...document.querySelectorAll('[contenteditable="true"]')].find(e => !e.closest('[data-pane-hidden]')) + if (!el) throw new Error('no composer') + el.focus() + for (let i = 0; i < 40; i++) { + const ch = 'the quick brown fox '[i % 20] + el.dispatchEvent(new KeyboardEvent('keydown', { bubbles: true, key: ch })) + el.textContent += ch + el.dispatchEvent(new InputEvent('input', { bubbles: true, data: ch, inputType: 'insertText' })) + el.dispatchEvent(new KeyboardEvent('keyup', { bubbles: true, key: ch })) + await new Promise(r => requestAnimationFrame(r)) + } +` + +// Session switching: click each sidebar session row, time route->settled. +const SWITCH = ` + (async () => { + const pick = () => [...document.querySelectorAll('[data-slot="row-button"]')] + .filter(el => el.offsetParent !== null).slice(0, 8) + if (pick().length < 2) { + throw new Error('found ' + pick().length + ' session rows') + } + const times = [] + const labels = [] + for (let i = 0; i < Math.min(pick().length, 6); i++) { + // Re-query each iteration: a switch can re-render the sidebar and + // detach the previously captured nodes. + const row = pick()[i] + if (!row) break + labels.push((row.textContent || '').slice(0, 24)) + const t0 = performance.now() + row.click() + let calm = 0 + while (calm < 2 && performance.now() - t0 < 5000) { + const f0 = performance.now() + await new Promise(r => requestAnimationFrame(r)) + const dt = performance.now() - f0 + calm = dt < 20 ? calm + 1 : 0 + } + times.push(Math.round(performance.now() - t0)) + await new Promise(r => setTimeout(r, 400)) + } + return { switches: times, labels, avg: Math.round(times.reduce((a, b) => a + b, 0) / times.length) } + })() +` + +const cmd = process.argv[2] ?? 'status' +const arg = process.argv[3] +const { evaluate, close } = await attach() + +try { + if (cmd === 'status') { + const r = await evaluate(`JSON.stringify({ + url: location.hash, + perfLive: typeof window.__PERF_LIVE__ !== 'undefined', + renderCounts: typeof window.__RENDER_COUNTS__ !== 'undefined', + tiles: document.querySelectorAll('[data-tree-group]').length, + sessions: document.querySelectorAll('[data-slot*="session-row"], [data-session-row]').length, + composers: [...document.querySelectorAll('[contenteditable="true"]')].length + })`) + console.log(r) + } else if (cmd === 'fps') { + console.log(JSON.stringify(await evaluate(FPS(Number(arg ?? 4))))) + } else if (cmd === 'drag') { + const r = await evaluate(WITH_LOAF(DRAG_BODY)) + console.log('drag', JSON.stringify({ fps: r.fps, p95: r.p95, worst: r.worst, slow33: r.slow33 })) + for (const lf of r.longFrames) { + console.log(` ⏱ ${lf.ms}ms block=${lf.block} style=${lf.style} → ${lf.scripts.join(' | ') || '(no script ≥5ms)'}`) + } + } else if (cmd === 'type') { + const r = await evaluate(WITH_LOAF(TYPE_BODY)) + console.log('type', JSON.stringify({ fps: r.fps, p95: r.p95, worst: r.worst, slow33: r.slow33 })) + for (const lf of r.longFrames) { + console.log(` ⏱ ${lf.ms}ms block=${lf.block} style=${lf.style} → ${lf.scripts.join(' | ') || '(no script ≥5ms)'}`) + } + } else if (cmd === 'switch') { + console.log(JSON.stringify(await evaluate(SWITCH))) + } else if (cmd === 'eval') { + console.log(JSON.stringify(await evaluate(arg))) + } else if (cmd === 'profile') { + // CPU-profile one session switch via CDP Profiler (Document Policy blocks + // the in-page Profiler API, CDP is exempt). arg = row label prefix. + await send('Profiler.enable') + await send('Profiler.setSamplingInterval', { interval: 200 }) + await send('Profiler.start') + const r = await evaluate(` + (async () => { + const pick = () => [...document.querySelectorAll('[data-slot="row-button"]')].filter(el => el.offsetParent !== null) + const row = pick().find(el => (el.textContent || '').startsWith(${JSON.stringify(arg ?? 'GUI')})) + if (!row) return 'row not found' + const t0 = performance.now() + row.click() + let calm = 0 + while (calm < 3 && performance.now() - t0 < 6000) { + const f0 = performance.now() + await new Promise(r => requestAnimationFrame(r)) + calm = (performance.now() - f0) < 20 ? calm + 1 : 0 + } + return Math.round(performance.now() - t0) + })() + `) + const { profile } = await send('Profiler.stop') + // Self-time per function. + const hitById = new Map() + for (let i = 0; i < profile.samples.length; i++) { + const id = profile.samples[i] + const dt = profile.timeDeltas[i] ?? 0 + hitById.set(id, (hitById.get(id) ?? 0) + dt) + } + const rows = [] + for (const node of profile.nodes) { + const us = hitById.get(node.id) + if (!us || us < 5000) continue + const f = node.callFrame + rows.push([Math.round(us / 1000), `${f.functionName || '(anon)'} @ ${(f.url || '').split('/').pop()}:${f.lineNumber}`]) + } + rows.sort((a, b) => b[0] - a[0]) + console.log('switch took', r, 'ms — top self-time:') + for (const [ms, name] of rows.slice(0, 18)) { + console.log(` ${String(ms).padStart(6)}ms ${name}`) + } + } else if (cmd === 'send') { + const r = await evaluate(` + (async () => { + const el = [...document.querySelectorAll('[contenteditable="true"]')].find(e => !e.closest('[data-pane-hidden]')) + if (!el) return 'no composer' + el.focus() + document.execCommand('insertText', false, ${JSON.stringify(arg ?? 'hello')}) + await new Promise(r => setTimeout(r, 120)) + el.dispatchEvent(new KeyboardEvent('keydown', { bubbles: true, cancelable: true, key: 'Enter' })) + return 'sent' + })() + `) + console.log(r) + } else { + console.log('unknown cmd', cmd) + } +} finally { + close() +} diff --git a/apps/desktop/src/app/chat/close-tab.ts b/apps/desktop/src/app/chat/close-tab.ts index 098e59108aea..b5e15dd30cf1 100644 --- a/apps/desktop/src/app/chat/close-tab.ts +++ b/apps/desktop/src/app/chat/close-tab.ts @@ -1,6 +1,7 @@ import { closeActiveTerminal } from '@/app/right-sidebar/terminal/terminals' import { closeWorkspaceTab } from '@/components/pane-shell/tree/store' import { isFocusWithin } from '@/lib/keybinds/combo' +import { $artifactTabs } from '@/store/artifacts' import { $filePreviewTabs, $previewTarget, closeActiveRightRailTab } from '@/store/preview' import { closeSessionTile, nextSessionTileForWorkspace } from '@/store/session-states' @@ -32,7 +33,7 @@ export function closeActiveTab(loadSessionIntoWorkspace?: (storedSessionId: stri // file tabs remain (the rail UI falls back to tabs[0]). Gating only on // `$filePreviewTarget` made ⌘W fall through to closeWorkspaceTab() and look // broken with a file tab still on screen. - if ($previewTarget.get() || $filePreviewTabs.get().length > 0) { + if ($previewTarget.get() || $filePreviewTabs.get().length > 0 || $artifactTabs.get().length > 0) { return closeActiveRightRailTab() } diff --git a/apps/desktop/src/app/chat/index.tsx b/apps/desktop/src/app/chat/index.tsx index 5fdf5a0487b1..40a8922d453f 100644 --- a/apps/desktop/src/app/chat/index.tsx +++ b/apps/desktop/src/app/chat/index.tsx @@ -71,7 +71,7 @@ interface ChatViewProps extends Omit, 'onSubmit'> { onCancel: () => Promise | void onAddContextRef: (refText: string, label?: string, detail?: string) => void onAddUrl: (url: string) => void - onBranchInNewChat: (messageId: string) => void + onBranchInNewChat?: (messageId: string) => void maxVoiceRecordingSeconds?: number onAttachImageBlob: (blob: Blob) => Promise | boolean | void onAttachDroppedItems: (candidates: DroppedFile[]) => Promise | boolean | void diff --git a/apps/desktop/src/app/chat/right-rail/artifact-pane.tsx b/apps/desktop/src/app/chat/right-rail/artifact-pane.tsx new file mode 100644 index 000000000000..c463ee84c41a --- /dev/null +++ b/apps/desktop/src/app/chat/right-rail/artifact-pane.tsx @@ -0,0 +1,222 @@ +import { useStore } from '@nanostores/react' +import { useEffect, useMemo, useState } from 'react' + +import { CopyButton } from '@/components/ui/copy-button' +import { Tip } from '@/components/ui/tooltip' +import { useI18n } from '@/i18n' +import { artifactDownloadName } from '@/lib/artifact-detect' +import { downloadTextFile } from '@/lib/download-text' +import { ChevronLeft, ChevronRight, Download, ExternalLink } from '@/lib/icons' +import { cn } from '@/lib/utils' +import { + $artifactRegistry, + $artifactVersionSelection, + type ArtifactRecord, + selectArtifactVersion +} from '@/store/artifacts' +import { notifyError } from '@/store/notifications' + +import { ArtifactLivePreview, ArtifactSourceView, composeArtifactHtml } from './artifact-renderers' +import { PreviewEmptyState } from './preview-file' + +type ArtifactViewMode = 'preview' | 'source' + +const MIME_BY_KIND = { code: 'text/plain', html: 'text/html', svg: 'image/svg+xml' } as const + +const HEADER_BUTTON_CLASS = + 'flex h-5 items-center gap-1 rounded-md px-1 text-[0.625rem] font-bold text-muted-foreground transition-colors hover:bg-accent hover:text-foreground disabled:pointer-events-none disabled:opacity-40' + +/** Write the composed document to a real temp file through the existing + * buffer-save IPC, then hand it to the OS browser. A blob/data URL can't + * cross into the OS default browser, so a file on disk is the honest path. */ +async function openHtmlInBrowser(content: string): Promise { + const bridge = window.hermesDesktop + + if (!bridge?.saveImageBuffer || !bridge.openExternal) { + throw new Error('Desktop bridge unavailable') + } + + const bytes = new TextEncoder().encode(composeArtifactHtml(content)) + const path = await bridge.saveImageBuffer(bytes, '.html') + + if (!path) { + throw new Error('Could not write artifact file') + } + + const fileUrl = `file://${path.startsWith('/') ? '' : '/'}${path.replace(/\\/g, '/')}` + + if (bridge.openPreviewInBrowser) { + await bridge.openPreviewInBrowser(fileUrl) + + return + } + + await bridge.openExternal(fileUrl) +} + +function VersionStepper({ + current, + onSelect, + total +}: { + current: number + onSelect: (index: number) => void + total: number +}) { + const { t } = useI18n() + const copy = t.artifactPane + + if (total < 2) { + return null + } + + return ( +
+ + + + {copy.versionOf(current + 1, total)} + + + +
+ ) +} + +export function ArtifactPane({ artifactId }: { artifactId: string }) { + const { t } = useI18n() + const copy = t.artifactPane + const registry = useStore($artifactRegistry) + const versionSelection = useStore($artifactVersionSelection) + // View mode is per-pane, ephemeral: renderable artifacts open in preview. + const [userMode, setUserMode] = useState(null) + + // Reset the explicit mode when the pane is reused for another artifact. + useEffect(() => { + setUserMode(null) + }, [artifactId]) + + const record = useMemo(() => { + for (const records of Object.values(registry)) { + const found = records.find(candidate => candidate.id === artifactId) + + if (found) { + return found + } + } + + return null + }, [artifactId, registry]) + + if (!record) { + return + } + + const isRenderable = record.kind === 'html' || record.kind === 'svg' + const versionIndex = Math.min(versionSelection[artifactId] ?? record.versions.length - 1, record.versions.length - 1) + const version = record.versions[versionIndex]! + const isCurrentVersion = versionIndex >= record.versions.length - 1 + const mode: ArtifactViewMode = isRenderable ? (userMode ?? 'preview') : 'source' + const downloadName = artifactDownloadName(record.kind, record.language, record.title) + + const modeLabel: Record = { + preview: copy.modePreview, + source: copy.modeSource + } + + return ( +
+
+
+ selectArtifactVersion(artifactId, index)} + total={record.versions.length} + /> + {!isCurrentVersion && ( + + )} +
+ {isRenderable && + (['preview', 'source'] as const).map(candidate => ( + + ))} +
+ + + + + {record.kind === 'html' && window.hermesDesktop && ( + + + + )} +
+
+
+ {mode === 'preview' && isRenderable ? ( + + ) : ( + + )} +
+
+ ) +} diff --git a/apps/desktop/src/app/chat/right-rail/artifact-renderers.tsx b/apps/desktop/src/app/chat/right-rail/artifact-renderers.tsx new file mode 100644 index 000000000000..00ed9a58a58b --- /dev/null +++ b/apps/desktop/src/app/chat/right-rail/artifact-renderers.tsx @@ -0,0 +1,107 @@ +import DOMPurify from 'dompurify' +import { useMemo } from 'react' +import ShikiHighlighter from 'react-shiki' + +import { chunkTextLines, useFixedRowWindow } from '@/components/chat/fixed-row-window' +import type { ArtifactKind } from '@/lib/artifact-detect' + +const SHIKI_THEME = { dark: 'github-dark-default', light: 'github-light-default' } as const +const SOURCE_CHUNK_LINES = 200 +const SOURCE_LINE_PX = 20 +const SOURCE_OVERSCAN_LINES = 400 + +/** Windowed, Shiki-highlighted source view for artifact content. Same fixed-row + * windowing as the file preview's SourceView so a 5k-line artifact scrolls + * smoothly, minus the gutter drag/selection machinery (artifact content has no + * on-disk path to reference lines against). */ +export function ArtifactSourceView({ language, text }: { language: string; text: string }) { + const chunks = useMemo(() => chunkTextLines(text, SOURCE_CHUNK_LINES), [text]) + const lastChunk = chunks.at(-1) + const totalLines = lastChunk ? lastChunk.start + lastChunk.lines.length : 0 + + const { afterRows, beforeRows, endChunk, onScroll, scrollerRef, startChunk } = useFixedRowWindow({ + overscanRows: SOURCE_OVERSCAN_LINES, + rowPx: SOURCE_LINE_PX, + rowsPerChunk: SOURCE_CHUNK_LINES, + totalRows: totalLines + }) + + const visibleChunks = chunks.slice(startChunk, endChunk + 1) + + return ( +
+
+ {beforeRows > 0 &&
} + {visibleChunks.map(chunk => ( +
+ + {chunk.text} + +
+ ))} + {afterRows > 0 &&
} +
+
+ ) +} + +/** Wrap an HTML fragment in a minimal document shell; full documents pass + * through untouched. Keeps generated fragments (no /) rendering + * with sane defaults instead of quirks-mode soup. */ +export function composeArtifactHtml(content: string): string { + if (/]|', + '', + '', + content, + '' + ].join('\n') +} + +/** + * Sandboxed live renderer for html/svg artifact content. + * + * HTML runs in an `