diff --git a/agent/coding_context.py b/agent/coding_context.py index 43fb33d4b7..b35e346998 100644 --- a/agent/coding_context.py +++ b/agent/coding_context.py @@ -412,7 +412,10 @@ def coding_compact_skill_categories(*, platform: Optional[str] = None, cwd: Opti def _git(cwd: Path, *args: str) -> str: """``git -C `` → stripped stdout, or ``""`` on any failure. bounded_git_probe bounds post-kill cleanup on Windows — plain ``subprocess.run(timeout=...)`` deadlocked - when a killed git left a suspended descendant holding the pipe handles.""" + when a killed git left a suspended descendant holding the pipe handles. + + See #66037. + """ return bounded_git_probe(["git", "-C", str(cwd), *args], timeout=_GIT_TIMEOUT) diff --git a/agent/status_output.py b/agent/status_output.py index ba04821e94..7ba5c71d75 100644 --- a/agent/status_output.py +++ b/agent/status_output.py @@ -44,7 +44,13 @@ class StatusOutputMixin: def _should_emit_quiet_tool_messages(self) -> bool: """True when quiet-mode tool summaries should print directly (CLI, no callback owns rendering); - ``suppress_status_output`` always wins so ``[tool]``/``[done]`` never land in captured stdout.""" + ``suppress_status_output`` always wins so ``[tool]``/``[done]`` never land in captured stdout. + + ``suppress_status_output`` (the strict machine-readable mode used by ``hermes chat -Q``) always + wins: those flows neutralize the rendering callbacks, and without this gate the "no callback owns + rendering" fallback would print ``[tool]``/``[done]`` spinner lines into the captured stdout it + exists to keep clean (#93220). + """ if getattr(self, "suppress_status_output", False): return False return self.quiet_mode and not self.tool_progress_callback and getattr(self, "platform", "") == "cli" @@ -92,7 +98,12 @@ class StatusOutputMixin: )) def _warn_uncompressed_context_overflow(self, preflight_tokens: int, context_length: int) -> None: - """Deduped warning when uncompressed context exceeds the model limit; points the user at /compact.""" + """Deduped warning when uncompressed context exceeds the model limit; points the user at /compact. + + When compression is explicitly disabled (compression.enabled: false), long sessions can grow past + the model context window with no compression to shrink them (#89297). Surface an actionable warning + so the user knows to run /compact or enable compression. + """ _warn_key = ("uncompressed_ctx_overflow", context_length) if getattr(self, "_last_ctx_overflow_warn", None) != _warn_key: self._last_ctx_overflow_warn = _warn_key diff --git a/agent/system_prompt.py b/agent/system_prompt.py index 3aca7e3b03..92370a6a93 100644 --- a/agent/system_prompt.py +++ b/agent/system_prompt.py @@ -185,7 +185,17 @@ def _session_start_like(agent: Any, now: Any) -> Any: lineage-root session id's embedded stamp (compaction rotates ids, each with its own mint time), the current session id's stamp, ``agent.session_start``, then ``now``. Stamps are box-local wall-clock: attach that zone first, then - convert to ``now``'s zone so the date matches the per-turn clock.""" + convert to ``now``'s zone so the date matches the per-turn clock. + + 0. the LINEAGE-ROOT session id's embedded timestamp — compaction can rotate the session id, and each + rotated id embeds its OWN mint time, so after months of compactions rung 1 alone would quietly re-birth + the conversation at its latest rotation. Walking to the lineage root (same walk as + ``_conversation_root_id``) recovers the ORIGINAL birth stamp — a Bot Mode forever-chat keeps knowing + when it was first born, across every compaction (maintainer-directed, #98426); 1. the timestamp embedded + in ``session_id`` (``YYYYMMDD_HHMMSS_...``) — immutable for the life of the session, so the line is + byte-stable across every rebuild boundary (preserving prefix-cache KV); 2. 3. ``now`` (initial/legacy + build without either). + """ from datetime import datetime def _to_display_tz(dt: Any) -> Any: if dt.tzinfo is None: @@ -221,7 +231,17 @@ def _agent_home(agent: Any) -> Optional[Path]: A bound HERMES_HOME ContextVar override wins (the gateway multiplexes profiles over one shared session DB and binds the home per turn); else the parent of ``_session_db.db_path`` — ground truth on threads that lost the - ContextVar, where ambient resolution would leak the launch profile.""" + ContextVar, where ambient resolution would leak the launch profile. + + 1. Surfaces that multiplex several profiles over ONE shared session DB (the messaging gateway: + ``gateway/run.py`` hands every agent the launch-home ``state.db`` and binds the profile home per turn + via ``_profile_runtime_scope`` + ``copy_context``) would otherwise have the db-derived launch home STOMP + the correctly-bound profile — inverting the leak this helper exists to fix (found by @kshitijk4poor's + post-merge probe on #86313). 2. Fallback: the home containing the agent's ``_session_db.db_path`` + (``/state.db``) — ground truth on threads that lost the ContextVar (ContextVars don't propagate + into ``threading.Thread``), where the unbound build previously fell back to the launch home and leaked + the default profile's skills/identity into a bot prompt. + """ try: from hermes_constants import get_hermes_home_override override = get_hermes_home_override() @@ -348,6 +368,11 @@ def _active_profile_line(agent: Any) -> str: # A non-default name is only returned when the resolved home is ALREADY # /profiles/, so the profile home is the session home itself. profile_home = str(_agent_home_path) if _agent_home_path is not None else str(get_hermes_home()) + # A non-default name is only ever returned when the resolved home is ALREADY /profiles/ — + # that is exactly how both _profile_name_for_home() and _resolve_active_profile_name() derive it. So the + # profile home is the session home itself; appending /profiles/ again doubled it (#72894). The + # default profile's data sits at the ROOT (get_default_hermes_root()), which in ambient profile mode is + # NOT get_hermes_home(). default_root = get_default_hermes_root() return ( f"Active Hermes profile: {active_profile}. This session reads " @@ -419,6 +444,13 @@ def _timestamp_line(agent: Any) -> str: _zone_suffix = f" ({', '.join(_bits)})" if _bits else "" _start = _session_start_like(agent, now) timestamp_line = f"Conversation started: {_start.strftime('%A, %B %d, %Y')}{_zone_suffix}" + # Second line (maintainer design, salvaging #96224's anchor): long-lived sessions — Bot Mode + # forever-chats, messenger channels people never close — span many days and many compactions. A lone + # birth date leads the model to believe it is still living in that old day. The prompt is rebuilt at + # every compaction boundary, so stamp the rebuild day too: 'started' stays anchored and byte-stable, 'as + # of' refreshes exactly when the cache prefix is already being invalidated (compaction), so the added + # line costs no extra cache churn. Same-day sessions skip the second line entirely — nothing to correct, + # and the single-line shape stays byte-identical for the day (prefix-cache safe). if now.strftime("%Y%m%d") != _start.strftime("%Y%m%d"): timestamp_line += (f"\nToday's date (as of the last context rebuild): {now.strftime('%A, %B %d, %Y')} " "— trust this over the start date for what day it is now; query tools for exact time.") @@ -439,6 +471,9 @@ def _memory_parts(agent: Any) -> List[str]: block = agent._memory_store.format_for_system_prompt(kind) if enabled else None if block: parts.append(block) + # External memory provider system prompt block (additive to built-in). Gated on the same check + # ``inject_memory_provider_tools`` uses so we never advertise provider tools that the agent's toolset + # configuration has already gated off (#81014). if agent._memory_manager: try: from agent.memory_manager import memory_provider_tools_exposed as _mem_exposed @@ -621,7 +656,15 @@ def build_system_prompt(agent: Any, system_message: Optional[str] = None) -> str def invalidate_system_prompt(agent: Any) -> None: """Force a rebuild on the next turn (after compression): reload memory from disk and clear the frozen plugin snapshot (previous bytes stashed as the - fail-open fallback) so plugins re-render at the same boundary.""" + fail-open fallback) so plugins re-render at the same boundary. + + Called after context compression events. Also reloads memory from disk so the rebuilt prompt captures + any writes from this session, and clears the frozen plugin-section snapshot so plugins re-render at the + same boundary (maintainer-directed, #95681 arc): a plugin section is just another prompt block carrying + state — freezing it while memory, skills, and guidance refresh would recreate the stale-block disease + inside plugin-land. The previous bytes are stashed so a plugin whose render RAISES falls back to its + last good section instead of vanishing (fail-open guard, not a freeze). + """ agent._cached_system_prompt = None agent._cached_system_prompt_static = None if hasattr(agent, "_plugin_system_prompt_sections_snapshot"): diff --git a/agent/title_generator.py b/agent/title_generator.py index b729670d50..eaeb450104 100644 --- a/agent/title_generator.py +++ b/agent/title_generator.py @@ -25,6 +25,7 @@ FailureCallback = Callable[[str, BaseException], None] TitleCallback = Callable[[str, str], None] # () -> bool, called right before the LLM request; False skips (e.g. the user switched models and # the request would reload one the runtime already evicted). +# Validation callback: () -> bool. See #19027. RuntimeValidator = Callable[[], bool] # Text budget handed to the model (Claude Code / OpenClaw converged on 1000). @@ -32,6 +33,10 @@ MAX_TITLE_INPUT_CHARS = 1000 # Cap on the instant derived title; a raw fragment reads worse the longer it runs. MAX_DERIVED_TITLE_CHARS = 48 # Answer-shaped guard: a tiny model sometimes answers instead of titling; longer is rejected, not truncated. +# Upper bound on accepted title word count. Titling is a 3-7 word task; a small tiny-model sometimes ignores +# the task and answers the user's message instead — that answer must never become the session title (see the +# answer-shaped output guard in generate_title; port of can1357/oh-my-pi#7306). 12 leaves headroom for +# legitimate wordy titles while excluding full-sentence answers. _MAX_TITLE_WORDS = 12 _TITLE_PROMPT_TEMPLATE = ( @@ -78,6 +83,11 @@ _MACHINE_PREFIXES = ( "[CONTEXT COMPACTION", LEGACY_SUMMARY_PREFIX, "[Runtime note:", "[System note:", "[SYSTEM]", # tui_gateway.server._MODEL_SWITCH_MARKER_PREFIX (keep in sync); persisted as role="user" because # strict providers reject a non-first system message. + # Model-switch marker from tui_gateway.server._append_model_switch_marker. It is persisted with + # role="user" (strict OpenAI-compatible providers reject a system message that is not first — #48338), + # so without this entry it looks like a real opening turn: switching models before the first real + # message titled the session "[System: The active model for this chat has…" instead of the user's actual + # question. "[System: The active model for this chat has changed to ", ) @@ -227,7 +237,12 @@ def generate_title( runtime_validator: Optional[RuntimeValidator] = None, ) -> Optional[str]: """Title from the opening message alone (waiting for the assistant made this slow and bought - nothing). ``runtime_validator`` runs right before the request; False skips silently.""" + nothing). ``runtime_validator`` runs right before the request; False skips silently. + + If it returns False (e.g. the user's model was switched since the background thread captured its runtime + snapshot), the call is skipped silently — no request is sent, so a stale title request can't reload a + model the runtime already unloaded (#19027). + """ if not _auto_title_enabled(): logger.debug("Auto-title skipped: auxiliary.title_generation.enabled=false") return None @@ -254,6 +269,11 @@ def generate_title( extra_body={"response_format": _TITLE_RESPONSE_FORMAT}, ) title = _clean_title(_extract_title_text(response.choices[0].message.content or "")) + # Answer-shaped output guard: titling is a 3-7 word task, so a title with many words is a model that + # ignored the task and answered the user's message instead ("I don't have context on X — that's not + # something I recognize..."). Truncating would store half an assistant blob as the session title, + # which is still an assistant blob — reject instead so the caller retries on the next exchange + # (maybe_auto_title fires for the first two exchanges). Port of can1357/oh-my-pi#7306. if title is not None and len(title.split()) > _MAX_TITLE_WORDS: # Answer-shaped output: reject (not truncate) so the caller retries next exchange. logger.debug("Rejecting answer-shaped title output (%d words > %d)", len(title.split()), _MAX_TITLE_WORDS) @@ -283,7 +303,11 @@ def _persist_session_title(session_db, session_id, title, *, source, dedupe=True transaction, so a manual ``/title`` is never overwritten); None when a higher authority held the row. ``ValueError`` = unique-title index collision → append ``#N`` via ``get_next_title_in_lineage``; ``dedupe=False`` re-raises instead (the derived title is on the critical path, collides constantly - on "hi", and the model replaces it a second later anyway).""" + on "hi", and the model replaces it a second later anyway). + + ``ValueError`` means the name is taken by an unrelated session (the unique-title index); rather than + leave the session untitled (#50537), append a ``#N`` suffix via ``get_next_title_in_lineage``. + """ auto_fn = getattr(session_db, "set_auto_title", None) def _set(candidate): @@ -348,6 +372,8 @@ def auto_title_session( with suppress(Exception): conversation_id = session_db.get_conversation_root(session_id) or session_id set_conversation_context(conversation_id) + # Same for the accounting context, so the title call's token usage is recorded against this session + # (task='title_generation', #23270). set_accounting_context(session_db, session_id) title, source = generate_title( user_message, failure_callback=failure_callback, main_runtime=main_runtime, runtime_validator=runtime_validator, diff --git a/agent/tool_executor.py b/agent/tool_executor.py index 42e05826ef..67c297001f 100644 --- a/agent/tool_executor.py +++ b/agent/tool_executor.py @@ -93,7 +93,13 @@ def _ensure_file_checkpoint(agent, function_name: str, function_args: dict, effe def _budget_for_agent(agent) -> BudgetConfig: """Tool-result BudgetConfig scaled to the agent's context window. Unknown length goes through ``budget_for_context_window(None)`` (not DEFAULT_BUDGET) so the MCP threshold - override still applies.""" + override still applies. + + Large-context models keep the historical 100K/200K char defaults; small models (e.g. a 65K-token local + model switched into mid-session) get a budget proportional to their window so a single large tool result + can't push the request past the model's limit (#23767). Falls back to the default budget when the + context length isn't resolvable. + """ try: ctx = getattr(getattr(agent, "context_compressor", None), "context_length", None) return budget_for_context_window(int(ctx) if ctx else None) @@ -114,10 +120,22 @@ def _authorization_gate_lock_timeout() -> float: """Authorization-lock bound = ``tools.approval.human_wait_ceiling`` (approval timeout + margin, capped so it can't overflow Lock.acquire): never break serialization while a prompt is answerable, never let a wedged holder park workers forever. Deliberately NOT - min()'d with the fallback so the gate never gives up early.""" + min()'d with the fallback so the gate never gives up early. + + Delegates to ``tools.approval.human_wait_ceiling`` — the same bound that clamps a human-wait window's + deadline contribution — so the two can't drift. Long enough that serialization is never broken while a + legitimate approval prompt is still answerable; short enough that a wedged holder (hanging + ``pre_tool_call`` plugin, dead approval client) cannot park other workers forever (#79719). Resolved + once per gate (per batch), so a mid-process ``approvals.timeout`` change applies from the next batch. + """ try: from tools.approval import human_wait_ceiling + # human_wait_ceiling is platform-safety-capped (agent/deadline.py MAX_SAFE_TIMEOUT_S): a huge + # approvals.timeout can no longer overflow Lock.acquire's time_t on macOS (#83220). Deliberately NOT + # min()'d with _AUTHORIZATION_GATE_LOCK_TIMEOUT_S — the gate must never give up while a legitimate + # approval prompt is still answerable (#79719), so a configured approvals.timeout above 360s must + # extend the gate. return human_wait_ceiling() except Exception: return _AUTHORIZATION_GATE_LOCK_TIMEOUT_S @@ -208,7 +226,12 @@ def _ra(): def _is_interpreter_shutdown_submit_error(exc: RuntimeError) -> bool: - """Shutdown-race predicate; ``tools.interpreter_shutdown`` knows both CPython message variants.""" + """Shutdown-race predicate; ``tools.interpreter_shutdown`` knows both CPython message variants. + + Delegates so all sites (cron delivery, conversation-loop retry, tool submission) recognize both CPython + shutdown-message variants instead of each matching its own substring (the bug class behind + #55924/#58720). + """ from tools.interpreter_shutdown import interpreter_shutting_down return interpreter_shutting_down(exc) @@ -431,6 +454,17 @@ class _ConcurrentToolAuthorizationGate: the batch behind a wedged plugin/approval client. Exclusion is measured at the SOURCE of the human wait (``tools.approval.human_wait_seconds``), NOT as gate residency — residency-based exclusion let a wedged plugin keep the deadline from ever firing. + + Serialization keeps concurrent approval prompts from interleaving on the user's screen. The acquire is + BOUNDED: a worker wedged inside the gate (a hanging ``pre_tool_call`` plugin, or an approval round-trip + to a client that went away) must not park every other worker forever. On expiry the worker runs its + prompt unserialized — worst case is interleaved prompts, strictly better than permanent starvation (same + tradeoff as the start-order gate, #79705). + Gate residency is arbitrary code — using it as the exclusion signal let a wedged plugin grow the + exclusion 1:1 with wall clock, keeping the batch deadline's ``remaining`` constant so it never fired and + the turn hung forever (#79719). A wedged plugin now contributes nothing to the exclusion and the batch + times out normally, while a genuine approval wait (which can legitimately exceed any fixed bound) is + still excluded in full. """ def __init__(self, *, lock_timeout: float | None = None, session_key: str | None = None) -> None: @@ -462,6 +496,13 @@ class _ConcurrentToolAuthorizationGate: def run(self, callback): if not self._serialization_lock.acquire(timeout=self._lock_timeout): + # Deterministic failure (bad command, non-MCP URL, 401/403): every retry hits the same wall. + # Park immediately instead of burning the retry ladder and spamming N identical warnings + # (#65673). Auth failures park here too rather than returning. Returning ends the run task, and + # with it the only listener on ``_reconnect_event`` — so a 401 on the very first connect left + # the server unrevivable for the life of the process, even after the user re-authenticated with + # ``hermes mcp login``. Parking keeps the task alive so the 300s self-probe (and an explicit + # /mcp refresh) can pick up fresh tokens. logger.warning( "authorization gate lock not acquired after %.1fs " "(holder wedged in a pre_tool_call plugin or approval " @@ -538,6 +579,10 @@ def _run_with_activity_heartbeat(agent, function_name: str, fn): """Run ``fn()`` under the activity heartbeat; covers both executor paths.""" stop = threading.Event() thread = threading.Thread( + # Keep the gateway turn-inactivity watchdog from abandoning a turn whose tool call runs silently for + # longer than the inactivity timeout (#84491): stamp activity periodically while the tool is in + # flight, not just at start/completion. Both the sequential and the concurrent paths funnel through + # here, so a single heartbeat covers every tool. target=_run_tool_activity_heartbeat, args=(agent, stop, f"tool running: {function_name}"), kwargs={"interval": _TOOL_ACTIVITY_HEARTBEAT_INTERVAL_S}, @@ -1107,6 +1152,8 @@ class _ConcurrentBatch: abandoned at the gate (the main thread already wrote this slot; emitting would double-report the tool_call_id).""" agent = self.agent + # Approval/sudo callbacks (thread-local) and the agent turn's ContextVars are propagated by + # propagate_context_to_thread() at the submit site below (GHSA-qg5c-hvr5-hjgr, #13617). start = time.time() blocked = dispatched = False try: diff --git a/agent/tool_guardrails.py b/agent/tool_guardrails.py index f3d49605e0..7a01869ca2 100644 --- a/agent/tool_guardrails.py +++ b/agent/tool_guardrails.py @@ -289,6 +289,11 @@ class ToolCallGuardrailController: self._halt_decision: ToolGuardrailDecision | None = None # Identical-call streak: CONSECUTIVE identical (tool, args, result) calls; any different call or # result resets it, so re-reads after edits and varied polling are never flagged. + # Identical-call loop-breaker state (agent.stall_guards): tracks the CONSECUTIVE streak of identical + # (tool, canonical args) calls whose results were also identical. Per-turn, like everything else + # here. NOTE: open PR #85352 (patrykkopycinski) tracks no-progress loops ACROSS turns via a + # detection window — a different mechanism from this per-turn consecutive streak. Coordinate future + # work there. self._identical_streak_sig: ToolCallSignature | None = None self._identical_streak_result_hash: str = "" self._identical_streak_count: int = 0 @@ -353,6 +358,12 @@ class ToolCallGuardrailController: # same_tool_failure counts DIFFERENT args on one tool; for failure-tolerant # tools a run of distinct red commands is diagnosis, not a loop — warn, never halt. if ( + # Hard-stop widening (#89069 / #100849 bundle): the per-turn no-progress BLOCK above only + # covers tools in idempotent_tools, so a model replaying the same successful + # `terminal`/`skill_view` call with a byte-identical result ran until the iteration budget. + # The consecutive-identical streak is tool-agnostic; when hard stops are enabled, halt at + # the same idempotent_no_progress threshold. Pollers stay exempt (an unchanged poll is + # progress). self.config.hard_stop_enabled and tool_name not in FAILURE_TOLERANT_TOOL_NAMES and same_count >= self.config.same_tool_failure_halt_after diff --git a/tools/image_source.py b/tools/image_source.py index 70d28a8db1..b0108e3a4e 100644 --- a/tools/image_source.py +++ b/tools/image_source.py @@ -87,6 +87,11 @@ def _guard_credential_read(host_target: Path, src: str) -> None: """Shared credential-read guard: refuse secret-bearing files (.env, auth.json) with a specific error. Guard import is best-effort; a real block always propagates.""" try: + # Shared credential-read guard (agent.file_safety, #57698): refuse secret-bearing files (.env, + # auth.json, ...) with an intentional, specific error instead of relying on the magic-byte sniff to + # reject them incidentally. Same chokepoint the image-gen/video-gen provider plugins enforce on + # model-supplied local paths. Import is best-effort (guard unavailability must not break image + # loading); a real block always propagates. from agent.file_safety import raise_if_read_blocked except Exception: # noqa: BLE001 — guard unavailable: proceed return @@ -190,7 +195,12 @@ def _get_active_env(task_id: Optional[str]): def _ensure_container_env(task_id: Optional[str]) -> None: """Lazily bring up the sandbox before an in-sandbox read (vision may be a session's first - action). Best-effort: failure leaves the env absent and the caller hits the fail-closed error.""" + action). Best-effort: failure leaves the env absent and the caller hits the fail-closed error. + + Unlike the terminal tool, vision never triggered environment creation, so a session whose first action + is ``vision_analyze`` on a container-only path under a non-local backend found no active env and failed + — until a terminal command happened to create one (issue #62825). + """ if not task_id: return try: @@ -208,8 +218,13 @@ async def _resolve_container_fallback( Cold-start retry: under Docker the first exec against a fresh container can fail (empty pipe) while a second succeeds. On final failure the container's output is folded into the error so "no such file" / "permission denied" / "never came up" are distinguishable. + + We retry once with a short delay before giving up, so callers don't see "could not read inside the + sandbox" on a file that is verifiably readable on the immediate retry. See #76566. """ import shlex + # Bring the sandbox up on demand: without this, the first vision_analyze of a session (before any + # terminal command) has no active env to read from under a non-local backend (issue #62825). _ensure_container_env(ctx.task_id) env = _get_active_env(ctx.task_id) if env is None: