From b66776d594660b393957116c7dfe6efb10a88d90 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 2 Sep 2026 18:49:11 -0700 Subject: [PATCH 01/10] refactor(agent/tool_executor): split executor god functions into phase helpers; unify observe/commit and start-gate handling --- agent/tool_executor.py | 1584 +++++++++++++++++----------------------- 1 file changed, 674 insertions(+), 910 deletions(-) diff --git a/agent/tool_executor.py b/agent/tool_executor.py index 8695e2ce8b..2e033ecdad 100644 --- a/agent/tool_executor.py +++ b/agent/tool_executor.py @@ -67,10 +67,7 @@ def _tc_name(tool_call: Any) -> str: def _record_persisted_path_for_stub(agent, tool_call_id: str, function_result) -> None: - """Record the spillover file path so a later result-reference stub can't dangle. - - Best-effort: bookkeeping never breaks tool execution. - """ + """Record the spillover file path so a later result-reference stub can't dangle (best-effort).""" try: if not isinstance(function_result, str): return @@ -81,17 +78,11 @@ def _record_persisted_path_for_stub(agent, tool_call_id: str, function_result) - logger.debug("persisted-path record for result stub failed: %s", exc) -def _ensure_file_checkpoint( - agent, - function_name: str, - function_args: dict, - effective_task_id: str, -) -> None: +def _ensure_file_checkpoint(agent, function_name: str, function_args: dict, effective_task_id: str) -> None: """Checkpoint the same workspace path that the file tool will mutate.""" file_path = function_args.get("path", "") if not file_path: return - # File tools resolve relative paths against the task's live cwd (differs from the # process cwd in Docker); resolve the same way before locating the project root. from tools.file_tools import _resolve_path_for_task @@ -102,11 +93,7 @@ def _ensure_file_checkpoint( def _budget_for_agent(agent) -> BudgetConfig: - """Resolve a tool-result BudgetConfig scaled to the agent's context window. - - Small-context models get a proportional budget so one large result can't overflow - the request (#23767); falls back to the default when context length is unknown. - """ + """Tool-result BudgetConfig scaled to the agent's context window (default when unknown).""" try: ctx = getattr(getattr(agent, "context_compressor", None), "context_length", None) # budget_for_context_window(None), not DEFAULT_BUDGET, so the MCP threshold @@ -118,29 +105,27 @@ def _budget_for_agent(agent) -> BudgetConfig: # Maximum number of concurrent worker threads for parallel tool execution. _MAX_TOOL_WORKERS = 8 _DEFAULT_IMAGE_PARALLEL_REQUESTS = 4 -# Generous ceiling for slow-but-valid tool work (large page fetches, slow -# remote backends) so the batch guard does not preempt a legitimate attempt. +# Generous ceiling for slow-but-valid tool work so the batch guard never preempts a legitimate attempt. _DEFAULT_CONCURRENT_TOOL_TIMEOUT_S = 420.0 -# Start-order gate wait bound: long enough for an approval round-trip, short enough -# that one wedged dispatch cannot starve the batch. +# Start-order gate wait: long enough for an approval round-trip, short enough that one +# wedged dispatch cannot starve the batch. _START_ORDER_GATE_TIMEOUT_S = 120.0 -# Fallback authorization-gate lock bound; the effective bound derives from -# approvals.timeout (see _authorization_gate_lock_timeout) since overstaying it means wedged. +# Fallback authorization-gate lock bound; the effective bound derives from approvals.timeout +# (see _authorization_gate_lock_timeout) since overstaying it means wedged. _AUTHORIZATION_GATE_LOCK_TIMEOUT_S = 360.0 def _authorization_gate_lock_timeout() -> float: """Bound for the authorization serialization lock: approval timeout + margin. - Delegates to ``tools.approval.human_wait_ceiling`` so the two bounds can't drift: - never break serialization while an approval prompt is answerable, but never let a - wedged holder park other workers forever (#79719). Resolved once per batch. + Delegates to ``tools.approval.human_wait_ceiling`` so the two bounds can't drift: never + break serialization while an approval prompt is answerable, never let a wedged holder + park other workers forever. Safety-capped so a huge approvals.timeout can't overflow + Lock.acquire; deliberately NOT min()'d with the fallback so the gate never gives up early. """ try: from tools.approval import human_wait_ceiling - # Safety-capped so a huge approvals.timeout can't overflow Lock.acquire (#83220); - # deliberately NOT min()'d with the fallback so the gate never gives up early (#79719). return human_wait_ceiling() except Exception: return _AUTHORIZATION_GATE_LOCK_TIMEOUT_S @@ -173,11 +158,8 @@ def _parse_tool_arguments(raw_arguments: Any) -> tuple[dict, Optional[str]]: def _resolve_concurrent_tool_timeout() -> float | None: - """Resolve the per-batch concurrent tool deadline via the unified resolver (#85125). - - ``timeouts.tools.concurrent_batch`` wins; ``HERMES_CONCURRENT_TOOL_TIMEOUT_S`` is the - legacy bridge; ``0``/negative disables the bound. - """ + """Per-batch concurrent deadline: ``timeouts.tools.concurrent_batch`` wins, + ``HERMES_CONCURRENT_TOOL_TIMEOUT_S`` is the legacy bridge, ``0``/negative disables.""" from agent.deadline import resolve_timeout return resolve_timeout( @@ -187,12 +169,7 @@ def _resolve_concurrent_tool_timeout() -> float | None: ) -def _flush_session_db_after_tool_progress( - agent, - messages: list, - *, - stage: str, -) -> bool: +def _flush_session_db_after_tool_progress(agent, messages: list, *, stage: str) -> bool: """Flush tool-call progress to the session DB before projecting it to any UI. Tool side effects can kill/restart the process before turn-end persistence runs. @@ -215,19 +192,14 @@ def _flush_session_db_after_tool_progress( def _image_generate_parallel_limit() -> int: - """Return the configured image-generation parallelism cap (conservative default; - backend bursts hit TTFB/rate-limit failures). - """ + """Configured image-generation parallelism cap (conservative default; backend bursts + hit TTFB/rate-limit failures).""" try: from hermes_cli.config import load_config cfg = load_config() or {} image_gen = cfg.get("image_gen") if isinstance(cfg, dict) else None - value = ( - image_gen.get("max_parallel_requests") - if isinstance(image_gen, dict) - else None - ) + value = image_gen.get("max_parallel_requests") if isinstance(image_gen, dict) else None except Exception: value = None @@ -243,10 +215,7 @@ def _max_workers_for_tool_batch(runnable_calls) -> int: if not runnable_calls: return 0 max_workers = _MAX_TOOL_WORKERS - if any( - (call[2] if len(call) >= 3 else None) == "image_generate" - for call in runnable_calls - ): + if any((call[2] if len(call) >= 3 else None) == "image_generate" for call in runnable_calls): max_workers = min(max_workers, _image_generate_parallel_limit()) return min(len(runnable_calls), max_workers) @@ -258,9 +227,7 @@ def _ra(): def _is_interpreter_shutdown_submit_error(exc: RuntimeError) -> bool: - """Shutdown-race predicate; delegates to ``tools.interpreter_shutdown`` so every site - recognizes both CPython shutdown-message variants (#55924/#58720). - """ + """Shutdown-race predicate; ``tools.interpreter_shutdown`` knows both CPython message variants.""" from tools.interpreter_shutdown import interpreter_shutting_down return interpreter_shutting_down(exc) @@ -277,15 +244,11 @@ def _emit_cancelled_terminal_post_tool_call( effective_task_id: str, tool_call_id: str, start_time: float, - reason: str = "user interrupt", - error_type: str = "keyboard_interrupt", middleware_trace: Optional[list[dict[str, Any]]] = None, ) -> str: + """Synthesize the ``cancelled`` result for a KeyboardInterrupt mid-tool and emit its terminal hook.""" result = json.dumps( - { - "error": f"Tool execution cancelled by {reason}", - "status": "cancelled", - }, + {"error": "Tool execution cancelled by user interrupt", "status": "cancelled"}, ensure_ascii=False, ) _emit_terminal_post_tool_call( @@ -297,13 +260,29 @@ def _emit_cancelled_terminal_post_tool_call( tool_call_id=tool_call_id, duration_ms=int((time.time() - start_time) * 1000), status="cancelled", - error_type=error_type, - error_message=f"Tool execution cancelled by {reason}", + error_type="keyboard_interrupt", + error_message="Tool execution cancelled by user interrupt", middleware_trace=list(middleware_trace or []), ) return result +def _emit_invalid_arguments_post_tool_call( + agent, *, function_name: str, function_args: dict, result: str, effective_task_id: str, tool_call_id: str +) -> None: + _emit_terminal_post_tool_call( + agent, + function_name=function_name, + function_args=function_args, + result=result, + effective_task_id=effective_task_id, + tool_call_id=tool_call_id, + status="error", + error_type="invalid_tool_arguments", + error_message="Tool arguments must be a valid JSON object", + ) + + def _append_skipped_tool_results( agent, messages: list, @@ -327,12 +306,7 @@ def _append_skipped_tool_results( for tc in tool_calls: name = _tc_name(tc) result = content.format(name=name) - messages.append(make_tool_result_message( - name, - result, - _pairing_tool_call_id(tc), - effect_disposition="none", - )) + messages.append(make_tool_result_message(name, result, _pairing_tool_call_id(tc), effect_disposition="none")) if hook_error_type is not None: _emit_terminal_post_tool_call( agent, @@ -346,16 +320,14 @@ def _append_skipped_tool_results( error_message="Tool execution skipped due to user interrupt", ) if flush_stage is not None: - flushed = _flush_session_db_after_tool_progress( - agent, messages, stage=f"{flush_stage} {name}" - ) + flushed = _flush_session_db_after_tool_progress(agent, messages, stage=f"{flush_stage} {name}") if not flushed and stop_on_flush_failure: return False return True def _tool_search_scoped_names(agent) -> frozenset: - """Return the deferrable tool names the session may invoke via tool_call. + """Deferrable tool names the session may invoke via ``tool_call``. The Tool Search unwrap bypasses the bridge's scope check in ``model_tools.handle_function_call``, so restricted sessions are validated against @@ -397,7 +369,7 @@ def _tool_search_scoped_names(agent) -> frozenset: def _canonical_tool_name(function_name: str) -> str: - """Map legacy tool-name aliases (2026-08 renames) BEFORE agent-loop dispatch.""" + """Map legacy tool-name aliases BEFORE agent-loop dispatch.""" from model_tools import _LEGACY_TOOL_ALIASES as _lta return _lta.get(function_name, function_name) @@ -410,12 +382,11 @@ def _unwrap_tool_search_call( Checkpointing, guardrails, plugin hooks and the activity feed must observe the real tool name, not the bridge. ``tool_call.function`` stays untouched for the transcript - and tool_call_id pairing. The unwrap bypasses - handle_function_call's scope check, so session toolset scope is enforced HERE. - Returns ``(name, args, scope_block)``; ``scope_block`` is the block message when - the underlying tool is out of scope or its args fail the deferred-schema probe - (``flatten_probe`` collapses the probe's JSON payload to one plain string for - callers that wrap the message in ``{"error": ...}``). + and tool_call_id pairing. The unwrap bypasses handle_function_call's scope check, so + session toolset scope is enforced HERE. Returns ``(name, args, scope_block)``; + ``scope_block`` is the block message when the underlying tool is out of scope or its + args fail the deferred-schema probe (``flatten_probe`` collapses the probe's JSON + payload to one plain string for callers that wrap the message in ``{"error": ...}``). """ scope_block: Optional[str] = None try: @@ -467,9 +438,7 @@ def _parse_tool_call(agent, tool_call, *, flatten_probe: bool = False) -> _Parse args, parse_error = _parse_tool_arguments(tool_call.function.arguments) if parse_error is not None: return _ParsedCall(tool_call, name, args, [], parse_error, None) - name, args, scope_block = _unwrap_tool_search_call( - agent, name, args, flatten_probe=flatten_probe - ) + name, args, scope_block = _unwrap_tool_search_call(agent, name, args, flatten_probe=flatten_probe) return _ParsedCall(tool_call, name, args, [], None, scope_block) @@ -498,25 +467,16 @@ class _ConcurrentToolAuthorizationGate: """Serialize policy prompts and exclude human approval waits from batch deadlines. The acquire is BOUNDED: on expiry the worker prompts unserialized rather than - starving the batch behind a wedged plugin/approval client (#79705). + starving the batch behind a wedged plugin/approval client. Deadline 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 (#79719). + exclusion let a wedged plugin keep the deadline from ever firing. """ - def __init__( - self, - *, - lock_timeout: float | None = None, - session_key: str | None = None, - ) -> None: + def __init__(self, *, lock_timeout: float | None = None, session_key: str | None = None) -> None: self._serialization_lock = threading.Lock() - self._lock_timeout = ( - _authorization_gate_lock_timeout() - if lock_timeout is None - else lock_timeout - ) + self._lock_timeout = _authorization_gate_lock_timeout() if lock_timeout is None else lock_timeout self._session_key = session_key if self._session_key is None: try: @@ -595,6 +555,16 @@ def _interrupt_worker_tids(agent, tids, *, reason=_NO_REASON) -> None: pass +def _set_worker_activity_callback(agent) -> None: + """The activity callback is thread-local: bind it on THIS thread so tool-layer heartbeats fire.""" + try: + from tools.environments.base import set_activity_callback + + set_activity_callback(agent._touch_activity) + except Exception: + pass + + # Heartbeat cadence; must stay far below the gateway turn-inactivity timeout # (default 1800s) so a silent-but-healthy tool never looks idle. _TOOL_ACTIVITY_HEARTBEAT_INTERVAL_S = 30.0 @@ -606,13 +576,9 @@ def _run_tool_activity_heartbeat( label: str, interval: float = _TOOL_ACTIVITY_HEARTBEAT_INTERVAL_S, ) -> None: - """Daemon thread that stamps ``agent._touch_activity`` every ``interval`` seconds - until ``stop_event`` is set. - - Keeps the gateway turn-inactivity watchdog (default 30 min) from abandoning a turn - whose tool runs silently. Wedged tools stay bounded by the tool layer's own timeouts. - """ - + """Daemon thread stamping ``agent._touch_activity`` every ``interval`` seconds until + ``stop_event`` is set, so the gateway inactivity watchdog never abandons a turn whose + tool runs silently. Wedged tools stay bounded by the tool layer's own timeouts.""" try: while not stop_event.wait(interval): agent._touch_activity(label) @@ -622,9 +588,7 @@ def _run_tool_activity_heartbeat( def _run_with_activity_heartbeat(agent, function_name: str, fn): - """Run ``fn()`` with a heartbeat so the gateway inactivity watchdog doesn't abandon a - silent-but-live turn (#84491); covers both executor paths. - """ + """Run ``fn()`` under the activity heartbeat; covers both executor paths.""" stop = threading.Event() thread = threading.Thread( target=_run_tool_activity_heartbeat, @@ -662,10 +626,7 @@ def _blocked_tool_result( else: result = agent._guardrail_block_result(guardrail_decision) error_type = "guardrail_block" - error_message = ( - getattr(guardrail_decision, "message", None) - or "Tool blocked by guardrail policy" - ) + error_message = getattr(guardrail_decision, "message", None) or "Tool blocked by guardrail policy" _emit_terminal_post_tool_call( agent, function_name=function_name, @@ -681,6 +642,97 @@ def _blocked_tool_result( return result +def _pre_tool_block(agent, function_name: str, final_args: dict, effective_task_id: str, tool_call_id: str, trace: list): + """Run ``pre_tool_call`` plugin hooks; returns ``(block_message, final_args)`` with any + hook-modified args applied. Hook failures never block.""" + try: + from hermes_cli.plugins import _dispatch_pre_tool_call_hooks + + block_msg, modified_args = _dispatch_pre_tool_call_hooks( + function_name, + final_args, + **tool_hook_ids(agent, effective_task_id, tool_call_id), + middleware_trace=list(trace), + ) + return block_msg, (final_args if modified_args is None else modified_args) + except Exception: + return None, final_args + + +def _dispatch_authorized_once( + agent, + state: _ManagedToolResult, + final_args: dict[str, Any], + *, + function_name: str, + effective_task_id: str, + tool_call_id: str, + execute, + scope_block: str | None, + display_index: int | None, + begin_execution, + authorization_gate: _ConcurrentToolAuthorizationGate | None, +) -> Any: + """Hermes policy (scope β†’ plugin pre-hooks β†’ guardrails) then the one real dispatch. + + ``begin_execution`` (concurrent start-order gate) is advanced exactly once on every + path so later-ordered workers keep moving; blocked calls advance it without a callback. + """ + def _advance_start_order(callback=None) -> None: + if begin_execution is not None: + begin_execution(callback) + elif callback is not None: + callback() + + block_message, block_error_type = scope_block, "tool_scope_block" + if block_message is None: + block_error_type = "plugin_block" + + def _resolve_pre_tool_block(): + return _pre_tool_block(agent, function_name, final_args, effective_task_id, tool_call_id, state.middleware_trace) + + block_message, final_args = ( + _resolve_pre_tool_block() if authorization_gate is None else authorization_gate.run(_resolve_pre_tool_block) + ) + state.args = final_args + + guardrail_decision = None + if block_message is None: + guardrail_decision = agent._tool_guardrails.before_call(function_name, final_args) + if guardrail_decision.allows_execution: + guardrail_decision = None + + if block_message is not None or guardrail_decision is not None: + _advance_start_order() + state.blocked = True + return _blocked_tool_result( + agent, + function_name=function_name, + final_args=final_args, + effective_task_id=effective_task_id, + tool_call_id=tool_call_id, + block_message=block_message, + block_error_type=block_error_type, + guardrail_decision=guardrail_decision, + trace=state.middleware_trace, + ) + + if function_name == "memory": + agent._turns_since_memory = 0 + elif function_name == "skill_manage": + agent._iters_since_skill = 0 + + _advance_start_order(lambda: _begin_tool_execution( + agent, + function_name=function_name, + function_args=final_args, + effective_task_id=effective_task_id, + tool_call_id=tool_call_id, + display_index=display_index, + )) + return _run_with_activity_heartbeat(agent, function_name, lambda: execute(final_args)) + + def _run_agent_tool_execution_middleware( agent, *, @@ -703,101 +755,28 @@ def _run_agent_tool_execution_middleware( ) trace = middleware_trace if middleware_trace is not None else [] - state = { - "args": function_args, - "middleware_trace": trace, - "blocked": False, - "dispatched": False, - } + state = _ManagedToolResult(result=None, args=function_args, middleware_trace=trace, blocked=False, dispatched=False) dispatch_lock = threading.Lock() def _authorized_dispatch(final_args: dict[str, Any]) -> Any: with dispatch_lock: - if state["dispatched"]: - raise RuntimeError( - "Hermes tool execution callback invoked more than once" - ) - state["dispatched"] = True - state["blocked"] = False - state["args"] = final_args - - def _begin() -> None: - _begin_tool_execution( - agent, - function_name=function_name, - function_args=final_args, - effective_task_id=effective_task_id, - tool_call_id=tool_call_id, - display_index=display_index, - ) - - def _advance_start_order(callback=None) -> None: - if begin_execution is None: - if callback is not None: - callback() - return - begin_execution(callback) - - block_message = scope_block - block_error_type = "tool_scope_block" - if block_message is None: - block_error_type = "plugin_block" - - def _resolve_pre_tool_block(): - nonlocal final_args - try: - from hermes_cli.plugins import _dispatch_pre_tool_call_hooks - - block_msg, modified_args = _dispatch_pre_tool_call_hooks( - function_name, - final_args, - **tool_hook_ids(agent, effective_task_id, tool_call_id), - middleware_trace=list(state["middleware_trace"]), - ) - if modified_args is not None: - final_args = modified_args - state["args"] = modified_args - return block_msg - except Exception: - return None - - block_message = ( - _resolve_pre_tool_block() - if authorization_gate is None - else authorization_gate.run(_resolve_pre_tool_block) - ) - - guardrail_decision = None - if block_message is None: - guardrail_decision = agent._tool_guardrails.before_call( - function_name, final_args - ) - if guardrail_decision.allows_execution: - guardrail_decision = None - - if block_message is not None or guardrail_decision is not None: - _advance_start_order() - state["blocked"] = True - return _blocked_tool_result( - agent, - function_name=function_name, - final_args=final_args, - effective_task_id=effective_task_id, - tool_call_id=tool_call_id, - block_message=block_message, - block_error_type=block_error_type, - guardrail_decision=guardrail_decision, - trace=state["middleware_trace"], - ) - - if function_name == "memory": - agent._turns_since_memory = 0 - elif function_name == "skill_manage": - agent._iters_since_skill = 0 - - _advance_start_order(_begin) - return _run_with_activity_heartbeat( - agent, function_name, lambda: execute(final_args) + if state.dispatched: + raise RuntimeError("Hermes tool execution callback invoked more than once") + state.dispatched = True + state.blocked = False + state.args = final_args + return _dispatch_authorized_once( + agent, + state, + final_args, + function_name=function_name, + effective_task_id=effective_task_id, + tool_call_id=tool_call_id, + execute=execute, + scope_block=scope_block, + display_index=display_index, + begin_execution=begin_execution, + authorization_gate=authorization_gate, ) def _hermes_pipeline(relay_args: dict[str, Any]) -> Any: @@ -807,24 +786,18 @@ def _run_agent_tool_execution_middleware( skip_relay=True, **tool_hook_ids(agent, effective_task_id, tool_call_id), ) - request_args = ( - request_result.payload - if isinstance(request_result.payload, dict) - else relay_args - ) + request_args = request_result.payload if isinstance(request_result.payload, dict) else relay_args trace.clear() trace.extend(request_result.trace) return run_tool_execution_middleware( function_name, request_args, - lambda next_args: _authorized_dispatch( - next_args if isinstance(next_args, dict) else request_args - ), + lambda next_args: _authorized_dispatch(next_args if isinstance(next_args, dict) else request_args), original_args=function_args, **tool_hook_ids(agent, effective_task_id, tool_call_id), ) - result, _relay_args = relay_tools.execute( + state.result, _relay_args = relay_tools.execute( function_name, function_args, _hermes_pipeline, @@ -836,13 +809,7 @@ def _run_agent_tool_execution_middleware( "tool_call_id": tool_call_id or "", }, ) - return _ManagedToolResult( - result=result, - args=state["args"], - middleware_trace=state["middleware_trace"], - blocked=bool(state["blocked"]), - dispatched=bool(state["dispatched"]), - ) + return state # Sequential wait-loop interrupt poll cadence: /stop lands within ~1s even when @@ -851,7 +818,7 @@ _SEQUENTIAL_INTERRUPT_POLL_SECONDS = 1.0 def _resolve_sequential_tool_timeout() -> float | None: - """Deadline for one sequential tool call (#85125 Phase 2a). + """Deadline for one sequential tool call. ``timeouts.tools.sequential_call`` wins; unset inherits the concurrent batch deadline so the two paths can't drift. ``0``/negative disables the bound. @@ -862,10 +829,7 @@ def _resolve_sequential_tool_timeout() -> float | None: """ from agent.deadline import resolve_timeout - return resolve_timeout( - "tools.sequential_call", - default=_resolve_concurrent_tool_timeout(), - ) + return resolve_timeout("tools.sequential_call", default=_resolve_concurrent_tool_timeout()) def _abandoned_sequential_result( @@ -899,13 +863,33 @@ def _abandoned_sequential_result( error_message=error_message, middleware_trace=list(trace), ) - return _ManagedToolResult( - result=result_cls(message), - args=function_args, - middleware_trace=trace, - blocked=False, - dispatched=True, - ) + return _ManagedToolResult(result=result_cls(message), args=function_args, middleware_trace=trace, blocked=False, dispatched=True) + + +def _poll_sequential_future(agent, future, function_name: str, deadline: float | None, started: float, authorization_gate) -> tuple[str, Any]: + """Wait for the worker in interrupt-poll slices, extending the deadline by human + approval wait. Returns ``("done", result)``, ``("timeout", None)`` or ``("interrupted", None)``. + + A disabled deadline still polls: this loop is what makes a non-cooperative tool + interruptible, so no deadline must not mean no interrupt checks. + """ + _last_heartbeat = 0 + while True: + wait_slice = _SEQUENTIAL_INTERRUPT_POLL_SECONDS + if deadline is not None: + remaining = deadline + authorization_gate.excluded_seconds() - time.monotonic() + if remaining <= 0: + return "timeout", None + wait_slice = min(wait_slice, remaining) + try: + return "done", future.result(timeout=wait_slice) + except concurrent.futures.TimeoutError: + if agent._interrupt_requested: + return "interrupted", None + elapsed = int(time.monotonic() - started) + if elapsed - _last_heartbeat >= 30: + _last_heartbeat = elapsed + agent._touch_activity(f"sequential tool running ({elapsed}s): {function_name}") def _run_sequential_tool_execution_middleware( @@ -920,7 +904,7 @@ def _run_sequential_tool_execution_middleware( display_index: int | None = None, middleware_trace: list[dict[str, Any]] | None = None, ) -> _ManagedToolResult: - """Run one sequential call with the concurrent executor's deadline. + """Run one sequential call on a worker thread under the concurrent executor's deadline. Interactive tools (``clarify``) own their wait via ``agent.clarify_timeout``; the generic deadline would report ``tool_timeout`` while the prompt is still live. @@ -947,75 +931,42 @@ def _run_sequential_tool_execution_middleware( def _run() -> _ManagedToolResult: with _registered_tool_worker(agent) as tid: worker_tid.append(tid) - return _run_agent_tool_execution_middleware( - agent, authorization_gate=authorization_gate, **kwargs - ) + return _run_agent_tool_execution_middleware(agent, authorization_gate=authorization_gate, **kwargs) + ident = dict( + function_name=function_name, + function_args=function_args, + effective_task_id=effective_task_id, + tool_call_id=tool_call_id, + middleware_trace=middleware_trace, + ) executor = DaemonThreadPoolExecutor(max_workers=1) future = executor.submit(propagate_context_to_thread(_run)) - # Disabled timeout still runs on the worker: this wait loop is what makes a - # non-cooperative tool interruptible, so no deadline must not mean no interrupt checks. deadline = time.monotonic() + timeout_s if timeout_s is not None else None started = time.monotonic() - timed_out = False - interrupted = False - _last_heartbeat = 0 + abandoned = False try: - while True: - wait_slice = _SEQUENTIAL_INTERRUPT_POLL_SECONDS - if deadline is not None: - remaining = ( - deadline + authorization_gate.excluded_seconds() - time.monotonic() - ) - if remaining <= 0: - timed_out = True - break - wait_slice = min(wait_slice, remaining) - try: - return future.result(timeout=wait_slice) - except concurrent.futures.TimeoutError: - if agent._interrupt_requested: - interrupted = True - break - elapsed = int(time.monotonic() - started) - if elapsed - _last_heartbeat >= 30: - _last_heartbeat = elapsed - agent._touch_activity( - f"sequential tool running ({elapsed}s): {function_name}" - ) - - if interrupted: + state, result = _poll_sequential_future(agent, future, function_name, deadline, started, authorization_gate) + if state == "done": + return result + if state == "interrupted": # Belt-and-braces: interrupt() already fans out to tracked worker # tids, but the worker may have registered after the fan-out ran. - _interrupt_worker_tids( - agent, worker_tid, reason=getattr(agent, "_tool_interrupt_reason", None) - ) - # Grace for a cooperative tool to notice its interrupt bit (mirrors the - # concurrent path's 3s). + _interrupt_worker_tids(agent, worker_tid, reason=getattr(agent, "_tool_interrupt_reason", None)) + # Grace for a cooperative tool to notice its interrupt bit (mirrors the concurrent path's 3s). concurrent.futures.wait([future], timeout=3.0) if future.done() and not future.cancelled(): return future.result() - timed_out = True # reuse the abandon-shutdown path in finally + abandoned = True # reuse the abandon-shutdown path in finally future.cancel() - interrupt_reason = ( - getattr(agent, "_tool_interrupt_reason", None) - or "interrupt requested" - ) - message = ( - f"[Tool execution cancelled β€” {function_name} was abandoned: " - f"{interrupt_reason}]" - ) + interrupt_reason = getattr(agent, "_tool_interrupt_reason", None) or "interrupt requested" + message = f"[Tool execution cancelled β€” {function_name} was abandoned: {interrupt_reason}]" logger.info( "sequential tool %s abandoned due to %s (%.1fs elapsed)", function_name, interrupt_reason, time.monotonic() - started, ) return _abandoned_sequential_result( - agent, - function_name=function_name, - function_args=function_args, - effective_task_id=effective_task_id, - tool_call_id=tool_call_id, - middleware_trace=middleware_trace, + agent, **ident, message=message, duration_ms=int((time.monotonic() - started) * 1000), status="cancelled", @@ -1026,22 +977,13 @@ def _run_sequential_tool_execution_middleware( # Only reachable when a deadline exists (interrupted returns above). assert timeout_s is not None - message = ( - f"Error executing tool '{function_name}': " - f"timed out after {timeout_s:.1f}s" - ) - logger.warning( - "sequential tool %s timed out after %.1fs", function_name, timeout_s - ) + abandoned = True + message = f"Error executing tool '{function_name}': timed out after {timeout_s:.1f}s" + logger.warning("sequential tool %s timed out after %.1fs", function_name, timeout_s) future.cancel() _interrupt_worker_tids(agent, worker_tid) return _abandoned_sequential_result( - agent, - function_name=function_name, - function_args=function_args, - effective_task_id=effective_task_id, - tool_call_id=tool_call_id, - middleware_trace=middleware_trace, + agent, **ident, message=message, duration_ms=int(timeout_s * 1000), status="timeout", @@ -1052,7 +994,7 @@ def _run_sequential_tool_execution_middleware( finally: # Never join a wedged worker. DaemonThreadPoolExecutor also keeps it out # of the stdlib atexit join, matching the concurrent timeout path. - executor.shutdown(wait=not timed_out, cancel_futures=timed_out) + executor.shutdown(wait=not abandoned, cancel_futures=abandoned) def _begin_tool_execution( @@ -1071,128 +1013,40 @@ def _begin_tool_execution( prefix = f"Tool {display_index}" if display_index is not None else "Tool" if agent.verbose_logging: print(f" πŸ“ž {prefix}: {function_name}({list(display_args.keys())})") - print( - agent._wrap_verbose( - "Args: ", json.dumps(display_args, indent=2, ensure_ascii=False) - ) - ) + print(agent._wrap_verbose("Args: ", json.dumps(display_args, indent=2, ensure_ascii=False))) else: - args_preview = ( - args_str[: agent.log_prefix_chars] + "..." - if len(args_str) > agent.log_prefix_chars - else args_str - ) - print( - f" πŸ“ž {prefix}: {function_name}({list(function_args.keys())}) - " - f"{args_preview}" - ) + args_preview = args_str[: agent.log_prefix_chars] + "..." if len(args_str) > agent.log_prefix_chars else args_str + print(f" πŸ“ž {prefix}: {function_name}({list(function_args.keys())}) - {args_preview}") agent._current_tool = function_name agent._touch_activity(f"executing tool: {function_name}") - try: - from tools.environments.base import set_activity_callback - - set_activity_callback(agent._touch_activity) - except Exception: - pass + _set_worker_activity_callback(agent) if agent.tool_progress_callback: try: preview = _build_tool_preview(function_name, display_args) - agent.tool_progress_callback( - "tool.started", function_name, preview, display_args - ) + agent.tool_progress_callback("tool.started", function_name, preview, display_args) except Exception as callback_error: logging.debug("Tool progress callback error: %s", callback_error) if agent.tool_start_callback: try: - agent.tool_start_callback( - tool_call_id, function_name, display_args - ) + agent.tool_start_callback(tool_call_id, function_name, display_args) except Exception as callback_error: logging.debug("Tool start callback error: %s", callback_error) if not agent._checkpoint_mgr.enabled: return - if function_name in {"write_file", "patch"}: - try: - _ensure_file_checkpoint( - agent, - function_name, - function_args, - effective_task_id, - ) - except Exception: - pass - elif function_name == "terminal": - try: + try: + if function_name in {"write_file", "patch"}: + _ensure_file_checkpoint(agent, function_name, function_args, effective_task_id) + elif function_name == "terminal": command = function_args.get("command", "") if _is_destructive_command(command): - cwd = function_args.get("workdir") or os.getenv( - "TERMINAL_CWD", os.getcwd() - ) - agent._checkpoint_mgr.ensure_checkpoint( - cwd, f"before terminal: {command[:60]}" - ) - except Exception: - pass - - -def _append_finalized_tool_result( - agent, - messages: list, - *, - function_name: str, - function_args: dict, - function_result, - tool_call_id: str, - effective_task_id: str, - budget: BudgetConfig, - effect_disposition=None, -): - """Persist/spill, hint, wrap and append one tool result; flush the session DB. - - Returns ``(function_result, tool_message, risk_metadata)`` β€” ``function_result`` is the - persisted/hinted content β€” or ``None`` when the incremental flush failed (the caller - must stop the batch). - """ - if not _is_multimodal_tool_result(function_result): - function_result = maybe_persist_tool_result( - content=function_result, - tool_name=function_name, - tool_use_id=tool_call_id, - env=get_active_env(effective_task_id), - config=budget, - ) - _record_persisted_path_for_stub(agent, tool_call_id, function_result) - - subdir_hints = agent._subdirectory_hints.check_tool_call(function_name, function_args) - if subdir_hints: - if _is_multimodal_tool_result(function_result): - # Append the hint to the text summary part so the model still sees it; - # don't touch the image blocks. - _append_subdir_hint_to_multimodal(function_result, subdir_hints) - else: - function_result += subdir_hints - - # Unwrap _multimodal dicts to an OpenAI-style content list; text-only servers - # get a string-safe fallback so a rejected image result never poisons history. - _tool_content = agent._tool_result_content_for_active_model(function_name, function_result) - tool_message = make_tool_result_message( - function_name, - _tool_content, - tool_call_id, - effect_disposition=effect_disposition, - ) - messages.append(tool_message) - if not _flush_session_db_after_tool_progress( - agent, - messages, - stage=f"tool result {function_name}", - ): - return None - return function_result, tool_message, tool_message.get("_tool_output_risk") + cwd = function_args.get("workdir") or os.getenv("TERMINAL_CWD", os.getcwd()) + agent._checkpoint_mgr.ensure_checkpoint(cwd, f"before terminal: {command[:60]}") + except Exception: + pass def _emit_tool_completed_progress(agent, function_name: str, *, duration: float, is_error: bool, result) -> None: @@ -1220,66 +1074,16 @@ def _emit_tool_complete_and_risk( except Exception as cb_err: logging.debug("Tool complete callback error: %s", cb_err) - if ( - risk_metadata is not None - and risk_metadata.get("risk") != "low" - and agent.tool_progress_callback - ): + if risk_metadata is not None and risk_metadata.get("risk") != "low" and agent.tool_progress_callback: try: agent.tool_progress_callback( - "tool.output_risk", - function_name, - None, - None, - tool_call_id=tool_call_id, - risk_metadata=risk_metadata, + "tool.output_risk", function_name, None, None, + tool_call_id=tool_call_id, risk_metadata=risk_metadata, ) except Exception as cb_err: logging.debug("Tool output risk callback error: %s", cb_err) -def _observe_tool_result( - agent, - *, - function_name: str, - function_args: dict, - function_result, - tool_call_id: str, - tool_duration: float, - is_error: bool, - blocked: bool, - error_preview: Callable[[Any], Any], - success_log_chars: Optional[int] = None, -): - """Guardrail-observe a result that actually ran, log its outcome, and feed the - turn-end file-mutation verifier. Blocked calls never ran, so they count as neither - failure nor success and are not observed. ``success_log_chars`` (sequential path) - also logs the completion line. Returns the (possibly annotated) result.""" - if not blocked: - function_result = agent._append_guardrail_observation( - function_name, - function_args, - function_result, - failed=is_error, - tool_call_id=tool_call_id, - ) - if is_error: - logger.warning( - "Tool %s returned error (%.2fs): %s", - function_name, tool_duration, error_preview(function_result), - ) - elif success_log_chars is not None: - logger.info("tool %s completed (%.2fs, %d chars)", function_name, tool_duration, success_log_chars) - if not blocked: - try: - agent._record_file_mutation_result( - function_name, function_args, function_result, is_error, - ) - except Exception as _ver_err: - logging.debug("file-mutation verifier record failed: %s", _ver_err) - return function_result - - def _commit_tool_result( agent, messages: list, @@ -1295,16 +1099,93 @@ def _commit_tool_result( blocked: bool, effect_disposition, ): - """Mark the tool done, append + flush its result, then project ``tool.completed``. + """Mark the tool done; persist/spill, hint, wrap and append its result; flush the + session DB; then project ``tool.completed``. Returns ``(persisted_result, display_result, risk_metadata)`` β€” ``display_result`` is - the pre-persist content for UI previews β€” or ``None`` when the flush failed. + the pre-persist content for UI previews β€” or ``None`` when the flush failed (the + caller must stop the batch). """ agent._current_tool = None _status_suffix = " (error)" if is_error else "" agent._touch_activity(f"tool completed: {function_name} ({tool_duration:.1f}s){_status_suffix}") - finalized = _append_finalized_tool_result( + persisted_result = function_result + if not _is_multimodal_tool_result(persisted_result): + persisted_result = maybe_persist_tool_result( + content=persisted_result, + tool_name=function_name, + tool_use_id=tool_call_id, + env=get_active_env(effective_task_id), + config=budget, + ) + _record_persisted_path_for_stub(agent, tool_call_id, persisted_result) + + subdir_hints = agent._subdirectory_hints.check_tool_call(function_name, function_args) + if subdir_hints: + if _is_multimodal_tool_result(persisted_result): + # Hint goes on the text summary part so the model still sees it; image blocks untouched. + _append_subdir_hint_to_multimodal(persisted_result, subdir_hints) + else: + persisted_result += subdir_hints + + # Unwrap _multimodal dicts to an OpenAI-style content list; text-only servers + # get a string-safe fallback so a rejected image result never poisons history. + _tool_content = agent._tool_result_content_for_active_model(function_name, persisted_result) + tool_message = make_tool_result_message(function_name, _tool_content, tool_call_id, effect_disposition=effect_disposition) + messages.append(tool_message) + if not _flush_session_db_after_tool_progress(agent, messages, stage=f"tool result {function_name}"): + return None + + if not blocked: + _emit_tool_completed_progress(agent, function_name, duration=tool_duration, is_error=is_error, result=function_result) + return persisted_result, function_result, tool_message.get("_tool_output_risk") + + +def _observe_and_commit_tool_result( + agent, + messages: list, + *, + function_name: str, + function_args: dict, + function_result, + tool_call_id: str, + effective_task_id: str, + budget: BudgetConfig, + tool_duration: float, + is_error: bool, + blocked: bool, + effect_disposition, + error_preview: Callable[[Any], Any], + success_log_chars: Optional[int] = None, + verbose_text: Callable[[Any], Any] = lambda result: result, +): + """Guardrail-observe a result that actually ran, log its outcome, feed the turn-end + file-mutation verifier, then ``_commit_tool_result`` it. + + Blocked calls never ran, so they count as neither failure nor success and are not + observed. ``success_log_chars`` (sequential path) also logs the completion line. + """ + if not blocked: + function_result = agent._append_guardrail_observation( + function_name, function_args, function_result, failed=is_error, tool_call_id=tool_call_id, + ) + if is_error: + logger.warning("Tool %s returned error (%.2fs): %s", function_name, tool_duration, error_preview(function_result)) + elif success_log_chars is not None: + logger.info("tool %s completed (%.2fs, %d chars)", function_name, tool_duration, success_log_chars) + if not blocked: + try: + agent._record_file_mutation_result(function_name, function_args, function_result, is_error) + except Exception as _ver_err: + logging.debug("file-mutation verifier record failed: %s", _ver_err) + + if agent.verbose_logging: + logging.debug("Tool %s completed in %.2fs", function_name, tool_duration) + _log_result = verbose_text(function_result) + logging.debug("Tool result (%d chars): %s", len(_log_result), _log_result) + + return _commit_tool_result( agent, messages, function_name=function_name, @@ -1313,18 +1194,11 @@ def _commit_tool_result( tool_call_id=tool_call_id, effective_task_id=effective_task_id, budget=budget, + tool_duration=tool_duration, + is_error=is_error, + blocked=blocked, effect_disposition=effect_disposition, ) - if finalized is None: - return None - persisted_result, _tool_message, risk_metadata = finalized - - if not blocked: - _emit_tool_completed_progress( - agent, function_name, - duration=tool_duration, is_error=is_error, result=function_result, - ) - return persisted_result, function_result, risk_metadata def _finalize_tool_batch(agent, messages: list, effective_task_id: str, num_tools: int, budget: BudgetConfig) -> None: @@ -1412,11 +1286,8 @@ class _StartOrderGate: return False if not in_order: logger.warning( - "start-order gate timed out for %s (order=%d next=%d); " - "proceeding out of order", - tool_name or "tool", - order, - self._next_order, + "start-order gate timed out for %s (order=%d next=%d); proceeding out of order", + tool_name or "tool", order, self._next_order, ) try: if callback is not None: @@ -1427,6 +1298,25 @@ class _StartOrderGate: return True +class _WorkerStartOnce: + """One worker's handle on the start-order gate: advances at most once, raising + ``_BatchAbandoned`` (instead of dispatching late) when the batch was abandoned.""" + + def __init__(self, gate: _StartOrderGate, order: int, tool_name: str) -> None: + self._gate, self._order, self._tool_name = gate, order, tool_name + self._advanced = False + + def advance(self, callback=None) -> None: + if self._advanced: + return + try: + proceed = self._gate.begin_in_order(self._order, callback, tool_name=self._tool_name) + finally: + self._advanced = True + if not proceed: + raise _BatchAbandoned(self._tool_name) + + class _ConcurrentBatch: """Shared state of one concurrent tool batch: per-slot results, the start-order and authorization gates, and the deadline bookkeeping the wait loop needs.""" @@ -1445,144 +1335,104 @@ class _ConcurrentBatch: self.authorization_gate = _ConcurrentToolAuthorizationGate() self.timed_out_indices: set[int] = set() - def run_worker( - self, - index, - tool_call, - function_name, - function_args, - middleware_trace, - scope_block, - start_order, - ): + def _dispatch_worker(self, index, tool_call_id, function_name, function_args, middleware_trace, scope_block, start_gate) -> Optional[_ToolOutcome]: + """Run one call through the middleware and synthesize its slot outcome; ``None`` + when the batch was abandoned at the gate (the main thread already wrote this slot, + so emitting anything would double-report the tool_call_id).""" + agent = self.agent + start = time.time() + blocked = dispatched = False + try: + managed = _run_agent_tool_execution_middleware( + agent, + function_name=function_name, + function_args=function_args, + effective_task_id=self.effective_task_id, + tool_call_id=tool_call_id, + execute=lambda next_args: agent._invoke_tool( + function_name, + next_args, + self.effective_task_id, + tool_call_id, + messages=self.messages, + pre_tool_block_checked=True, + skip_tool_request_middleware=True, + skip_tool_execution_middleware=True, + tool_request_middleware_trace=list(middleware_trace), + ), + scope_block=scope_block, + display_index=index + 1, + middleware_trace=middleware_trace, + begin_execution=start_gate.advance, + authorization_gate=self.authorization_gate, + ) + result, function_args, middleware_trace = managed.result, managed.args, managed.middleware_trace + blocked, dispatched = managed.blocked, managed.dispatched + except _BatchAbandoned: + logger.info("tool %s abandoned at start-order gate; skipping dispatch", function_name) + return None + except KeyboardInterrupt: + try: + agent.interrupt("keyboard interrupt") + except Exception: + pass + result = _emit_cancelled_terminal_post_tool_call( + agent, + function_name=function_name, + function_args=function_args, + effective_task_id=self.effective_task_id, + tool_call_id=tool_call_id, + start_time=start, + middleware_trace=list(middleware_trace), + ) + duration = time.time() - start + logger.info("tool %s cancelled (%.2fs)", function_name, duration) + return _ToolOutcome(function_name, function_args, result, duration, True, False, middleware_trace) + except Exception as tool_error: + result = f"Error executing tool '{function_name}': {tool_error}" + logger.error("_invoke_tool raised for %s: %s", function_name, tool_error, exc_info=True) + duration = time.time() - start + if not blocked and not dispatched: + _emit_terminal_post_tool_call( + agent, + function_name=function_name, + function_args=function_args, + result=result, + effective_task_id=self.effective_task_id, + tool_call_id=tool_call_id, + duration_ms=int(duration * 1000), + middleware_trace=list(middleware_trace), + ) + is_error, _ = _detect_tool_failure(function_name, result) + if is_error: + logger.info("tool %s failed (%.2fs): %s", function_name, duration, result[:200]) + else: + logger.info("tool %s completed (%.2fs, %d chars)", function_name, duration, len(result)) + return _ToolOutcome(function_name, function_args, result, duration, is_error, blocked, middleware_trace) + + def run_worker(self, index, tool_call, function_name, function_args, middleware_trace, scope_block, start_order): """Worker function executed in a thread.""" agent = self.agent with _registered_tool_worker(agent) as _worker_tid: - # Race: interrupt may have fanned out before our registration; apply it - # to our own tid now. + # Race: interrupt may have fanned out before our registration; apply it to our own tid now. if agent._interrupt_requested: - _interrupt_worker_tids( - agent, [_worker_tid], reason=getattr(agent, "_tool_interrupt_reason", None) - ) - # Activity callback is thread-local; set it on THIS worker so - # _wait_for_process heartbeats fire. - try: - from tools.environments.base import set_activity_callback - set_activity_callback(agent._touch_activity) - except Exception: - pass + _interrupt_worker_tids(agent, [_worker_tid], reason=getattr(agent, "_tool_interrupt_reason", None)) + _set_worker_activity_callback(agent) # Approval/sudo callbacks and turn ContextVars are propagated by - # propagate_context_to_thread() at submit (GHSA-qg5c-hvr5-hjgr, #13617). - start = time.time() - tool_call_id = _pairing_tool_call_id(tool_call) - blocked = False - dispatched = False - start_advanced = False - - def _advance_start(callback=None) -> None: - nonlocal start_advanced - if start_advanced: - return - try: - proceed = self.gate.begin_in_order(start_order, callback, tool_name=function_name) - finally: - start_advanced = True - if not proceed: - # Batch already abandoned: the turn synthesized this tool's - # result and moved on. Abort instead of dispatching late. - raise _BatchAbandoned(function_name) - + # propagate_context_to_thread() at submit. + start_gate = _WorkerStartOnce(self.gate, start_order, function_name) try: - try: - def _execute(next_args: dict[str, Any]) -> Any: - return agent._invoke_tool( - function_name, - next_args, - self.effective_task_id, - tool_call_id, - messages=self.messages, - pre_tool_block_checked=True, - skip_tool_request_middleware=True, - skip_tool_execution_middleware=True, - tool_request_middleware_trace=list(middleware_trace), - ) - - managed = _run_agent_tool_execution_middleware( - agent, - function_name=function_name, - function_args=function_args, - effective_task_id=self.effective_task_id, - tool_call_id=tool_call_id, - execute=_execute, - scope_block=scope_block, - display_index=index + 1, - middleware_trace=middleware_trace, - begin_execution=_advance_start, - authorization_gate=self.authorization_gate, - ) - result = managed.result - function_args = managed.args - middleware_trace = managed.middleware_trace - blocked = managed.blocked - dispatched = managed.dispatched - except _BatchAbandoned: - # Abandoned at the start-order gate: the main thread already synthesized - # this result, so write/emit nothing (would double-report the tool_call_id). - logger.info( - "tool %s abandoned at start-order gate; skipping dispatch", - function_name, - ) - return - except KeyboardInterrupt: - try: - agent.interrupt("keyboard interrupt") - except Exception: - pass - result = _emit_cancelled_terminal_post_tool_call( - agent, - function_name=function_name, - function_args=function_args, - effective_task_id=self.effective_task_id, - tool_call_id=tool_call_id, - start_time=start, - middleware_trace=list(middleware_trace), - ) - duration = time.time() - start - logger.info("tool %s cancelled (%.2fs)", function_name, duration) - self.results[index] = _ToolOutcome( - function_name, function_args, result, duration, True, False, middleware_trace, - ) - return - except Exception as tool_error: - result = f"Error executing tool '{function_name}': {tool_error}" - logger.error("_invoke_tool raised for %s: %s", function_name, tool_error, exc_info=True) - duration = time.time() - start - if not blocked and not dispatched: - _emit_terminal_post_tool_call( - agent, - function_name=function_name, - function_args=function_args, - result=result, - effective_task_id=self.effective_task_id, - tool_call_id=tool_call_id, - duration_ms=int(duration * 1000), - middleware_trace=list(middleware_trace), - ) - is_error, _ = _detect_tool_failure(function_name, result) - if is_error: - logger.info("tool %s failed (%.2fs): %s", function_name, duration, result[:200]) - else: - logger.info("tool %s completed (%.2fs, %d chars)", function_name, duration, len(result)) - self.results[index] = _ToolOutcome( - function_name, function_args, result, duration, is_error, blocked, middleware_trace, + outcome = self._dispatch_worker( + index, _pairing_tool_call_id(tool_call), function_name, function_args, + middleware_trace, scope_block, start_gate, ) + if outcome is not None: + self.results[index] = outcome finally: # Teardown advance keeps later-ordered workers moving; never let the # abandonment signal escape here. - try: - _advance_start() - except _BatchAbandoned: - pass + with contextlib.suppress(_BatchAbandoned): + start_gate.advance() def submit_all(self, executor, runnable_calls) -> tuple[list, dict]: """Submit every runnable call; on interpreter shutdown, synthesize error results @@ -1595,21 +1445,14 @@ class _ConcurrentBatch: try: f = executor.submit( propagate_context_to_thread(self.run_worker), - i, - tc, - name, - args, - self.parsed_calls[i].middleware_trace, - scope_block, - submit_index, + i, tc, name, args, self.parsed_calls[i].middleware_trace, scope_block, submit_index, ) except RuntimeError as submit_error: if not _is_interpreter_shutdown_submit_error(submit_error): raise skipped_calls = runnable_calls[submit_index:] logger.warning( - "interpreter shutdown while scheduling concurrent tools; " - "skipping %d unsubmitted tool(s)", + "interpreter shutdown while scheduling concurrent tools; skipping %d unsubmitted tool(s)", len(skipped_calls), ) for skipped_i, _tc, skipped_name, skipped_args, _scope_block in skipped_calls: @@ -1628,11 +1471,7 @@ class _ConcurrentBatch: return futures, future_to_index def _running_names(self, not_done, future_to_index) -> list[str]: - return [ - self.parsed_calls[future_to_index[f]].name - for f in not_done - if f in future_to_index - ] + return [self.parsed_calls[future_to_index[f]].name for f in not_done if f in future_to_index] def await_completion(self, futures, future_to_index, deadline: float | None) -> bool: """Wait with periodic heartbeats (gateway inactivity monitor) and interrupt checks @@ -1655,16 +1494,10 @@ class _ConcurrentBatch: if not not_done: return False - if ( - deadline is not None - and time.monotonic() >= deadline + self.authorization_gate.excluded_seconds() - ): - self.timed_out_indices = { - future_to_index[f] for f in not_done if f in future_to_index - } + if deadline is not None and time.monotonic() >= deadline + self.authorization_gate.excluded_seconds(): + self.timed_out_indices = {future_to_index[f] for f in not_done if f in future_to_index} logger.warning( - "concurrent tool batch timed out after %.1fs; " - "%d tool(s) still running: %s", + "concurrent tool batch timed out after %.1fs; %d tool(s) still running: %s", self.timeout_s, len(self.timed_out_indices), ", ".join(self._running_names(not_done, future_to_index)[:5]), @@ -1683,17 +1516,15 @@ class _ConcurrentBatch: # completion; cancel unstarted futures so we don't block on them. if agent._interrupt_requested: agent._vprint( - f"{agent.log_prefix}⚑ Interrupt: cancelling " - f"{len(not_done)} pending concurrent tool(s)", + f"{agent.log_prefix}⚑ Interrupt: cancelling {len(not_done)} pending concurrent tool(s)", force=True, ) for f in not_done: f.cancel() - # Release gate-parked workers so they abort instead of - # dispatching after the turn was already interrupted. + # Release gate-parked workers so they abort instead of dispatching after + # the turn was already interrupted; then give running tools a moment to + # notice the per-thread interrupt signal and exit gracefully. self.gate.abandon() - # Give already-running tools a moment to notice the - # per-thread interrupt signal and exit gracefully. concurrent.futures.wait(not_done, timeout=3.0) return True @@ -1732,10 +1563,7 @@ class _ConcurrentBatch: self.gate.abandon() # On abandon do NOT join hung workers: a wedged thread is left detached # rather than deadlocking the batch. Normal completion joins. - executor.shutdown( - wait=not abandon_executor, - cancel_futures=abandon_executor, - ) + executor.shutdown(wait=not abandon_executor, cancel_futures=abandon_executor) def _unfinished_tool_result( @@ -1752,49 +1580,30 @@ def _unfinished_tool_result( Returns ``(function_result, tool_duration, effect_disposition)``. """ - def _emit(result: str, *, status: str, error_type: str, error_message: str, duration_ms: int = 0) -> None: - _emit_terminal_post_tool_call( - agent, - function_name=pc.name, - function_args=pc.args, - result=result, - effective_task_id=effective_task_id, - tool_call_id=tool_call_id, - duration_ms=duration_ms, - status=status, - error_type=error_type, - error_message=error_message, - middleware_trace=list(pc.middleware_trace), - ) - if timed_out: suffix = f"{timeout_s:.1f}s" if timeout_s is not None else "the configured timeout" function_result = f"Error executing tool '{pc.name}': timed out after {suffix}" - _emit( - function_result, - duration_ms=int((timeout_s or 0.0) * 1000), - status="timeout", - error_type="tool_timeout", - error_message=function_result, - ) - return function_result, float(timeout_s or 0.0), "unknown" - if agent._interrupt_requested: + outcome = dict(duration_ms=int((timeout_s or 0.0) * 1000), status="timeout", error_type="tool_timeout", error_message=function_result) + tool_duration, effect_disposition = float(timeout_s or 0.0), "unknown" + elif agent._interrupt_requested: function_result = f"[Tool execution cancelled β€” {pc.name} was skipped due to user interrupt]" - _emit( - function_result, - status="cancelled", - error_type="keyboard_interrupt", - error_message="Tool execution cancelled by user interrupt", - ) + outcome = dict(status="cancelled", error_type="keyboard_interrupt", error_message="Tool execution cancelled by user interrupt") + tool_duration, effect_disposition = 0.0, None else: function_result = f"Error executing tool '{pc.name}': thread did not return a result" - _emit( - function_result, - status="error", - error_type="thread_missing_result", - error_message=function_result, - ) - return function_result, 0.0, None + outcome = dict(status="error", error_type="thread_missing_result", error_message=function_result) + tool_duration, effect_disposition = 0.0, None + _emit_terminal_post_tool_call( + agent, + function_name=pc.name, + function_args=pc.args, + result=function_result, + effective_task_id=effective_task_id, + tool_call_id=tool_call_id, + middleware_trace=list(pc.middleware_trace), + **outcome, + ) + return function_result, tool_duration, effect_disposition def _append_batch_results(agent, messages: list, effective_task_id: str, batch: _ConcurrentBatch, budget: BudgetConfig) -> bool: @@ -1806,7 +1615,7 @@ def _append_batch_results(agent, messages: list, effective_task_id: str, batch: # A worker may finish between the deadline snapshot and this loop; # prefer its real result over a fabricated timeout. if r is None: - name, args, middleware_trace, is_error, blocked = pc.name, pc.args, pc.middleware_trace, True, False + name, args = pc.name, pc.args function_result, tool_duration, effect_disposition = _unfinished_tool_result( agent, pc, tool_call_id=tool_call_id, @@ -1814,61 +1623,35 @@ def _append_batch_results(agent, messages: list, effective_task_id: str, batch: timed_out=i in batch.timed_out_indices, timeout_s=batch.timeout_s, ) - else: - name, args, function_result, tool_duration, is_error, blocked, middleware_trace = ( - r.name, r.args, r.result, r.duration, r.is_error, r.blocked, r.middleware_trace, + committed = _commit_tool_result( + agent, messages, + function_name=name, function_args=args, function_result=function_result, + tool_call_id=tool_call_id, effective_task_id=effective_task_id, budget=budget, + tool_duration=tool_duration, is_error=True, blocked=False, + effect_disposition=effect_disposition, ) + blocked = False + else: + name, args, tool_duration, blocked = r.name, r.args, r.duration, r.blocked if pc.parse_error is not None: - _emit_terminal_post_tool_call( - agent, - function_name=name, - function_args=args, - result=function_result, - effective_task_id=effective_task_id, - tool_call_id=tool_call_id, - status="error", - error_type="invalid_tool_arguments", - error_message="Tool arguments must be a valid JSON object", - middleware_trace=list(middleware_trace), + _emit_invalid_arguments_post_tool_call( + agent, function_name=name, function_args=args, result=r.result, + effective_task_id=effective_task_id, tool_call_id=tool_call_id, ) - effect_disposition = "none" if blocked else None - function_result = _observe_tool_result( - agent, - function_name=name, - function_args=args, - function_result=function_result, - tool_call_id=tool_call_id, - tool_duration=tool_duration, - is_error=is_error, - blocked=blocked, + committed = _observe_and_commit_tool_result( + agent, messages, + function_name=name, function_args=args, function_result=r.result, + tool_call_id=tool_call_id, effective_task_id=effective_task_id, budget=budget, + tool_duration=tool_duration, is_error=r.is_error, blocked=blocked, + effect_disposition="none" if blocked else None, error_preview=lambda res: _multimodal_text_summary(res)[:200], ) - if agent.verbose_logging: - logging.debug("Tool %s completed in %.2fs", name, tool_duration) - logging.debug("Tool result (%d chars): %s", len(function_result), function_result) - - committed = _commit_tool_result( - agent, - messages, - function_name=name, - function_args=args, - function_result=function_result, - tool_call_id=tool_call_id, - effective_task_id=effective_task_id, - budget=budget, - tool_duration=tool_duration, - is_error=is_error, - blocked=blocked, - effect_disposition=effect_disposition, - ) if committed is None: return False _persisted, display_function_result, risk_metadata = committed if agent._should_emit_quiet_tool_messages(): - cute_msg = _get_cute_tool_message_impl( - name, args, tool_duration, result=display_function_result, - ) + cute_msg = _get_cute_tool_message_impl(name, args, tool_duration, result=display_function_result) agent._safe_print(f" {cute_msg}") elif _tool_progress_enabled(agent): _print_tool_completed(agent, i + 1, tool_duration, _multimodal_text_summary(display_function_result)) @@ -1897,7 +1680,6 @@ def execute_tool_calls_concurrent(agent, assistant_message, messages: list, effe # Resolve the context-scaled tool-output budget once per turn, not per result. _tool_budget = _budget_for_agent(agent) - # ── Pre-flight: interrupt check ────────────────────────────────── if agent._interrupt_requested: print(f"{agent.log_prefix}⚑ Interrupt: skipping {num_tools} tool call(s)") _append_skipped_tool_results( @@ -1920,18 +1702,11 @@ def execute_tool_calls_concurrent(agent, assistant_message, messages: list, effe timeout_s = _resolve_concurrent_tool_timeout() batch = _ConcurrentBatch(agent, messages, effective_task_id, parsed_calls, timeout_s) - # Touch activity before launching workers so the gateway knows - # we're executing tools (not stuck). + # Touch activity before launching workers so the gateway knows we're executing tools. agent._current_tool = tool_names_str agent._touch_activity(f"executing {num_tools} tools concurrently: {tool_names_str}") - # Start spinner for CLI mode (skip when TUI handles tool progress) - spinner = None - if agent._should_emit_quiet_tool_messages() and agent._should_start_quiet_spinner(): - face = random.choice(KawaiiSpinner.get_waiting_faces()) - spinner = KawaiiSpinner(f"{face} ⚑ running {num_tools} tools concurrently", spinner_type='dots', print_fn=agent._print_fn) - spinner.start() - + spinner = _start_quiet_tool_spinner(agent, "", {}, label=f"⚑ running {num_tools} tools concurrently") try: batch.run() finally: @@ -1984,11 +1759,7 @@ def _delegate_spinner_label(function_args: dict) -> str: if tasks_arg and isinstance(tasks_arg, list): return f"πŸ”€ delegating {len(tasks_arg)} tasks Β· (/agents to monitor)" goal_preview = (function_args.get("goal") or "")[:30] - return ( - f"πŸ”€ {goal_preview} Β· (/agents to monitor)" - if goal_preview - else "πŸ”€ delegating Β· (/agents to monitor)" - ) + return f"πŸ”€ {goal_preview} Β· (/agents to monitor)" if goal_preview else "πŸ”€ delegating Β· (/agents to monitor)" @dataclass @@ -2027,19 +1798,13 @@ def _resolve_sequential_dispatch( if function_name != "delegate_task" and function_name in INLINE_TOOL_EXECUTORS: # Agent-level tools that need live AIAgent state; table shared with invoke_tool. inline_executor = INLINE_TOOL_EXECUTORS[function_name] - inline_ctx = InlineToolContext( - effective_task_id=effective_task_id, - tool_call_id=tool_call_id, - messages=messages, - ) + inline_ctx = InlineToolContext(effective_task_id=effective_task_id, tool_call_id=tool_call_id, messages=messages) return _SequentialDispatch( execute=lambda next_args: inline_executor(agent, next_args, inline_ctx), finish_in_finally=False, ) if function_name == "delegate_task": - spinner = _start_quiet_tool_spinner( - agent, function_name, function_args, label=_delegate_spinner_label(function_args) - ) + spinner = _start_quiet_tool_spinner(agent, function_name, function_args, label=_delegate_spinner_label(function_args)) agent._delegate_spinner = spinner return _SequentialDispatch( execute=lambda next_args: agent._dispatch_delegate_task(next_args), @@ -2076,13 +1841,8 @@ def _resolve_sequential_dispatch( tool_call_id=tool_call_id, session_id=agent.session_id or "", turn_id=getattr(agent, "_current_turn_id", "") or "", - api_request_id=getattr(agent, "_current_api_request_id", "") - or "", - enabled_tools=( - list(agent.valid_tool_names) - if agent.valid_tool_names - else None - ), + api_request_id=getattr(agent, "_current_api_request_id", "") or "", + enabled_tools=list(agent.valid_tool_names) if agent.valid_tool_names else None, skip_pre_tool_call_hook=True, skip_tool_request_middleware=True, skip_tool_execution_middleware=True, @@ -2102,6 +1862,155 @@ def _resolve_sequential_dispatch( ) +def _skip_remaining_sequential(agent, messages: list, remaining, effective_task_id: str, *, notice: str, **skip_kwargs) -> bool: + """Announce an interrupt and append one skipped result per unstarted call; False when + a flush failed (the caller must stop the batch).""" + agent._vprint(f"{agent.log_prefix}⚑ Interrupt: skipping {len(remaining)} {notice}", force=True) + return _append_skipped_tool_results(agent, messages, remaining, effective_task_id, **skip_kwargs) + + +def _append_invalid_arguments_result(agent, messages: list, pc: _ParsedCall, *, tool_call_id: str, effective_task_id: str) -> bool: + """Emit + append the parse-error result for a call whose arguments were not a JSON object.""" + _emit_invalid_arguments_post_tool_call( + agent, function_name=pc.name, function_args=pc.args, result=pc.parse_error, + effective_task_id=effective_task_id, tool_call_id=tool_call_id, + ) + messages.append(make_tool_result_message(pc.name, pc.parse_error, tool_call_id)) + return _flush_session_db_after_tool_progress(agent, messages, stage=f"invalid tool arguments {pc.name}") + + +def _run_sequential_call( + agent, + dispatch: _SequentialDispatch, + pc: _ParsedCall, + *, + messages: list, + remaining_calls, + effective_task_id: str, + tool_call_id: str, + display_index: int, + tool_start_time: float, +) -> tuple[_ManagedToolResult, float]: + """Run one sequential call with its spinner/error policy; returns ``(managed, duration)``. + + KeyboardInterrupt (registry tools only) emits results for THIS and every remaining + call before re-raising so the tool-call turn keeps matching results (alternation). + """ + function_name, function_args, middleware_trace = pc.name, pc.args, pc.middleware_trace + _spinner_result = None + try: + managed = _run_sequential_tool_execution_middleware( + agent, + function_name=function_name, + function_args=function_args, + effective_task_id=effective_task_id, + tool_call_id=tool_call_id, + execute=dispatch.execute, + scope_block=pc.scope_block, + display_index=display_index, + middleware_trace=dispatch.middleware_trace_arg, + ) + function_args = managed.args + _spinner_result = managed.result + except KeyboardInterrupt: + if not dispatch.handles_keyboard_interrupt: + raise + _spinner_result = _emit_cancelled_terminal_post_tool_call( + agent, + function_name=function_name, + function_args=function_args, + effective_task_id=effective_task_id, + tool_call_id=tool_call_id, + start_time=tool_start_time, + middleware_trace=list(middleware_trace), + ) + try: + agent.interrupt("keyboard interrupt") + except Exception: + pass + _append_skipped_tool_results( + agent, messages, remaining_calls, effective_task_id, + content="[Tool execution cancelled β€” {name} was skipped due to keyboard interrupt]", + ) + raise + except Exception as tool_error: + if dispatch.error_result is None: + raise + function_result = dispatch.error_result(tool_error) + logger.error(dispatch.error_log, function_name, tool_error, exc_info=True) + managed = _ManagedToolResult(result=function_result, args=function_args, middleware_trace=middleware_trace, blocked=False, dispatched=False) + finally: + if dispatch.is_delegate: + agent._delegate_spinner = None + tool_duration = time.time() - tool_start_time + if dispatch.finish_spinner and dispatch.finish_in_finally: + _finish_quiet_tool_spinner(agent, dispatch.spinner, function_name, function_args, tool_duration, _spinner_result) + if dispatch.finish_spinner and not dispatch.finish_in_finally: + _finish_quiet_tool_spinner(agent, dispatch.spinner, function_name, function_args, tool_duration, _spinner_result) + return managed, tool_duration + + +def _publish_sequential_result( + agent, + messages: list, + *, + function_name: str, + managed: _ManagedToolResult, + tool_duration: float, + index: int, + tool_call_id: str, + effective_task_id: str, + budget: BudgetConfig, +) -> bool: + """Terminal hook β†’ observe β†’ commit β†’ completion callbacks/print for one sequential + result; False when the incremental flush failed (the caller must stop the batch).""" + function_args, function_result, middleware_trace = managed.args, managed.result, managed.middleware_trace + _execution_timed_out = isinstance(function_result, (_ToolTimeoutResult, _ToolCancelledResult)) + # Multimodal dict results (_multimodal=True) are not sliceable as strings. + _result_len = len(function_result) if isinstance(function_result, str) else len(str(function_result)) + _is_error_result, _ = _detect_tool_failure(function_name, function_result) + # Inline-dispatched runtime tools never reach handle_function_call, so the + # executor owns the one terminal post_tool_call per tool_call_id (the inner + # observer is suppressed); also stops an abandoned timeout worker reporting late. + if not managed.blocked and not _execution_timed_out: + _emit_terminal_post_tool_call( + agent, + function_name=function_name, + function_args=function_args, + result=function_result, + effective_task_id=effective_task_id, + tool_call_id=tool_call_id, + duration_ms=int(tool_duration * 1000), + middleware_trace=list(middleware_trace), + ) + committed = _observe_and_commit_tool_result( + agent, messages, + function_name=function_name, function_args=function_args, function_result=function_result, + tool_call_id=tool_call_id, effective_task_id=effective_task_id, budget=budget, + tool_duration=tool_duration, is_error=_is_error_result, blocked=managed.blocked, + effect_disposition="unknown" if _execution_timed_out else None, + error_preview=lambda res: res[:200] if isinstance(res, str) and not agent.verbose_logging else res, + success_log_chars=_result_len, + verbose_text=_multimodal_text_summary, + ) + if committed is None: + return False + function_result, display_function_result, risk_metadata = committed + + _emit_tool_complete_and_risk( + agent, + function_name=function_name, + function_args=function_args, + tool_call_id=tool_call_id, + result=display_function_result, + risk_metadata=risk_metadata, + blocked=managed.blocked, + ) + if _tool_progress_enabled(agent): + _print_tool_completed(agent, index, tool_duration, function_result) + return True + + def execute_tool_calls_sequential(agent, assistant_message, messages: list, effective_task_id: str, api_call_count: int = 0, *, finalize: bool = True) -> None: """Execute tool calls sequentially (single calls or interactive tools). @@ -2119,11 +2028,9 @@ def execute_tool_calls_sequential(agent, assistant_message, messages: list, effe # SAFETY: check interrupt BEFORE each tool so a "stop" during the previous # tool skips all remaining ones. if agent._interrupt_requested: - remaining_calls = tool_calls[i-1:] - if remaining_calls: - agent._vprint(f"{agent.log_prefix}⚑ Interrupt: skipping {len(remaining_calls)} tool call(s)", force=True) - if not _append_skipped_tool_results( - agent, messages, remaining_calls, effective_task_id, + if not _skip_remaining_sequential( + agent, messages, tool_calls[i - 1:], effective_task_id, + notice="tool call(s)", content="[Tool execution cancelled β€” {name} was skipped due to user interrupt]", hook_error_type="user_interrupt", hook_id=lambda tc: getattr(tc, "id", "") or "", @@ -2133,188 +2040,48 @@ def execute_tool_calls_sequential(agent, assistant_message, messages: list, effe break pc = _parse_tool_call(agent, tool_call, flatten_probe=True) - function_name, function_args = pc.name, pc.args if pc.parse_error is not None: - _emit_terminal_post_tool_call( - agent, - function_name=function_name, - function_args=function_args, - result=pc.parse_error, - effective_task_id=effective_task_id, - tool_call_id=tool_call_id, - status="error", - error_type="invalid_tool_arguments", - error_message="Tool arguments must be a valid JSON object", - ) - messages.append( - make_tool_result_message( - function_name, - pc.parse_error, - tool_call_id, - ) - ) - if not _flush_session_db_after_tool_progress( - agent, - messages, - stage=f"invalid tool arguments {function_name}", - ): + if not _append_invalid_arguments_result(agent, messages, pc, tool_call_id=tool_call_id, effective_task_id=effective_task_id): return continue - middleware_trace: list[dict[str, Any]] = pc.middleware_trace - _execution_blocked = False - _execution_dispatched = False - tool_start_time = time.time() - # One bounded execution funnel for every runtime-tool branch; no duplicated # timeout policy in the callbacks. dispatch = _resolve_sequential_dispatch( agent, - function_name=function_name, - function_args=function_args, + function_name=pc.name, + function_args=pc.args, messages=messages, effective_task_id=effective_task_id, tool_call_id=tool_call_id, - middleware_trace=middleware_trace, + middleware_trace=pc.middleware_trace, ) - _spinner_result = None - try: - managed = _run_sequential_tool_execution_middleware( - agent, - function_name=function_name, - function_args=function_args, - effective_task_id=effective_task_id, - tool_call_id=tool_call_id, - execute=dispatch.execute, - scope_block=pc.scope_block, - display_index=i, - middleware_trace=dispatch.middleware_trace_arg, - ) - function_result = managed.result - function_args = managed.args - middleware_trace = managed.middleware_trace - _execution_blocked = managed.blocked - _execution_dispatched = managed.dispatched - _spinner_result = function_result - except KeyboardInterrupt: - if not dispatch.handles_keyboard_interrupt: - raise - function_result = _emit_cancelled_terminal_post_tool_call( - agent, - function_name=function_name, - function_args=function_args, - effective_task_id=effective_task_id, - tool_call_id=tool_call_id, - start_time=tool_start_time, - middleware_trace=list(middleware_trace), - ) - _spinner_result = function_result - try: - agent.interrupt("keyboard interrupt") - except Exception: - pass - # Emit results for THIS and every remaining call before re-raising so - # the tool-call turn keeps matching results (alternation). - _append_skipped_tool_results( - agent, messages, tool_calls[i - 1:], effective_task_id, - content="[Tool execution cancelled β€” {name} was skipped due to keyboard interrupt]", - ) - raise - except Exception as tool_error: - if dispatch.error_result is None: - raise - function_result = dispatch.error_result(tool_error) - logger.error(dispatch.error_log, function_name, tool_error, exc_info=True) - finally: - if dispatch.is_delegate: - agent._delegate_spinner = None - tool_duration = time.time() - tool_start_time - if dispatch.finish_spinner and dispatch.finish_in_finally: - _finish_quiet_tool_spinner(agent, dispatch.spinner, function_name, function_args, tool_duration, _spinner_result) - if dispatch.finish_spinner and not dispatch.finish_in_finally: - _finish_quiet_tool_spinner(agent, dispatch.spinner, function_name, function_args, tool_duration, _spinner_result) - - _execution_timed_out = isinstance( - function_result, (_ToolTimeoutResult, _ToolCancelledResult) - ) - # Multimodal dict results (_multimodal=True) are not sliceable as strings. - _result_len = len(function_result) if isinstance(function_result, str) else len(str(function_result)) - - # Log tool errors to the persistent error log so [error] tags - # in the UI always have a corresponding detailed entry on disk. - _is_error_result, _ = _detect_tool_failure(function_name, function_result) - # Inline-dispatched runtime tools never reach handle_function_call, so the - # executor owns the one terminal post_tool_call per tool_call_id (the inner - # observer is suppressed); also stops an abandoned timeout worker reporting late. - if not _execution_blocked and not _execution_timed_out: - _emit_terminal_post_tool_call( - agent, - function_name=function_name, - function_args=function_args, - result=function_result, - effective_task_id=effective_task_id, - tool_call_id=tool_call_id, - duration_ms=int(tool_duration * 1000), - middleware_trace=list(middleware_trace), - ) - function_result = _observe_tool_result( - agent, - function_name=function_name, - function_args=function_args, - function_result=function_result, + managed, tool_duration = _run_sequential_call( + agent, dispatch, pc, + messages=messages, + remaining_calls=tool_calls[i - 1:], + effective_task_id=effective_task_id, tool_call_id=tool_call_id, - tool_duration=tool_duration, - is_error=_is_error_result, - blocked=_execution_blocked, - error_preview=lambda res: ( - res if agent.verbose_logging or not isinstance(res, str) - else (res[:200] if len(res) > 200 else res) - ), - success_log_chars=_result_len, + display_index=i, + tool_start_time=tool_start_time, ) - - if agent.verbose_logging: - logging.debug("Tool %s completed in %.2fs", function_name, tool_duration) - _log_result = _multimodal_text_summary(function_result) - logging.debug("Tool result (%d chars): %s", len(_log_result), _log_result) - - committed = _commit_tool_result( - agent, - messages, - function_name=function_name, - function_args=function_args, - function_result=function_result, + if not _publish_sequential_result( + agent, messages, + function_name=pc.name, + managed=managed, + tool_duration=tool_duration, + index=i, tool_call_id=tool_call_id, effective_task_id=effective_task_id, budget=_tool_budget, - tool_duration=tool_duration, - is_error=_is_error_result, - blocked=_execution_blocked, - effect_disposition="unknown" if _execution_timed_out else None, - ) - if committed is None: + ): return - function_result, display_function_result, risk_metadata = committed - - _emit_tool_complete_and_risk( - agent, - function_name=function_name, - function_args=function_args, - tool_call_id=tool_call_id, - result=display_function_result, - risk_metadata=risk_metadata, - blocked=_execution_blocked, - ) - - if _tool_progress_enabled(agent): - _print_tool_completed(agent, i, tool_duration, function_result) if agent._interrupt_requested and i < len(tool_calls): - remaining = len(tool_calls) - i - agent._vprint(f"{agent.log_prefix}⚑ Interrupt: skipping {remaining} remaining tool call(s)", force=True) - if not _append_skipped_tool_results( + if not _skip_remaining_sequential( agent, messages, tool_calls[i:], effective_task_id, + notice="remaining tool call(s)", content="[Tool execution skipped β€” {name} was not started. User sent a new message]", flush_stage="skipped tool result", ): @@ -2346,10 +2113,7 @@ def execute_tool_calls_segmented(agent, assistant_message, messages: list, effec return segment_message = SimpleNamespace(tool_calls=list(calls)) run_segment = execute_tool_calls_concurrent if kind == "parallel" else execute_tool_calls_sequential - run_segment( - agent, segment_message, messages, effective_task_id, api_call_count, - finalize=False, - ) + run_segment(agent, segment_message, messages, effective_task_id, api_call_count, finalize=False) if getattr(agent, "_incremental_persistence_failed", False): return From 12aebbfcbcf7a53d2ef2f06c8f274c36b54221ba Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 2 Sep 2026 18:59:32 -0700 Subject: [PATCH 02/10] refactor(agent/tool_executor): _ToolCallRef identity carrier + emit_post funnel replace 9 hand-rolled terminal-hook blocks --- agent/tool_executor.py | 528 ++++++++++++++--------------------------- 1 file changed, 173 insertions(+), 355 deletions(-) diff --git a/agent/tool_executor.py b/agent/tool_executor.py index 2e033ecdad..f4e8b90d9f 100644 --- a/agent/tool_executor.py +++ b/agent/tool_executor.py @@ -236,51 +236,53 @@ def _is_interpreter_shutdown_submit_error(exc: RuntimeError) -> bool: _emit_terminal_post_tool_call = emit_terminal_post_tool_call -def _emit_cancelled_terminal_post_tool_call( - agent, - *, - function_name: str, - function_args: dict, - effective_task_id: str, - tool_call_id: str, - start_time: float, - middleware_trace: Optional[list[dict[str, Any]]] = None, -) -> str: - """Synthesize the ``cancelled`` result for a KeyboardInterrupt mid-tool and emit its terminal hook.""" - result = json.dumps( - {"error": "Tool execution cancelled by user interrupt", "status": "cancelled"}, - ensure_ascii=False, - ) - _emit_terminal_post_tool_call( - agent, - function_name=function_name, - function_args=function_args, - result=result, - effective_task_id=effective_task_id, - tool_call_id=tool_call_id, - duration_ms=int((time.time() - start_time) * 1000), - status="cancelled", - error_type="keyboard_interrupt", - error_message="Tool execution cancelled by user interrupt", - middleware_trace=list(middleware_trace or []), - ) - return result +@dataclass +class _ToolCallRef: + """Identity of one tool call as every hook / result message sees it: the (possibly + middleware-rewritten) name and args, the task, the pairing id and the request trace.""" + name: str + args: dict + task_id: str + call_id: str + trace: list -def _emit_invalid_arguments_post_tool_call( - agent, *, function_name: str, function_args: dict, result: str, effective_task_id: str, tool_call_id: str -) -> None: - _emit_terminal_post_tool_call( - agent, - function_name=function_name, - function_args=function_args, - result=result, - effective_task_id=effective_task_id, - tool_call_id=tool_call_id, - status="error", - error_type="invalid_tool_arguments", - error_message="Tool arguments must be a valid JSON object", - ) + def emit_post(self, agent, result, *, trace=None, **outcome) -> None: + """Emit the one terminal ``post_tool_call`` for this call (``outcome`` = status / + error_type / error_message / duration_ms). Resolved through the module attribute so + tests patching ``_emit_terminal_post_tool_call`` still intercept.""" + _emit_terminal_post_tool_call( + agent, + function_name=self.name, + function_args=self.args, + result=result, + effective_task_id=self.task_id, + tool_call_id=self.call_id, + middleware_trace=list(self.trace if trace is None else trace), + **outcome, + ) + + def emit_cancelled(self, agent, start_time: float) -> str: + """Synthesize the ``cancelled`` result for a KeyboardInterrupt mid-tool and emit its hook.""" + result = json.dumps( + {"error": "Tool execution cancelled by user interrupt", "status": "cancelled"}, + ensure_ascii=False, + ) + self.emit_post( + agent, result, + duration_ms=int((time.time() - start_time) * 1000), + status="cancelled", + error_type="keyboard_interrupt", + error_message="Tool execution cancelled by user interrupt", + ) + return result + + def emit_invalid_arguments(self, agent, result: str) -> None: + self.emit_post( + agent, result, trace=[], + status="error", error_type="invalid_tool_arguments", + error_message="Tool arguments must be a valid JSON object", + ) def _append_skipped_tool_results( @@ -308,16 +310,9 @@ def _append_skipped_tool_results( result = content.format(name=name) messages.append(make_tool_result_message(name, result, _pairing_tool_call_id(tc), effect_disposition="none")) if hook_error_type is not None: - _emit_terminal_post_tool_call( - agent, - function_name=name, - function_args={}, - result=result, - effective_task_id=effective_task_id, - tool_call_id=(hook_id or _pairing_tool_call_id)(tc), - status="cancelled", - error_type=hook_error_type, - error_message="Tool execution skipped due to user interrupt", + _ToolCallRef(name, {}, effective_task_id, (hook_id or _pairing_tool_call_id)(tc), []).emit_post( + agent, result, + status="cancelled", error_type=hook_error_type, error_message="Tool execution skipped due to user interrupt", ) if flush_stage is not None: flushed = _flush_session_db_after_tool_progress(agent, messages, stage=f"{flush_stage} {name}") @@ -432,6 +427,9 @@ class _ParsedCall: parse_error: Optional[str] scope_block: Optional[str] + def ref(self, task_id: str) -> _ToolCallRef: + return _ToolCallRef(self.name, self.args, task_id, _pairing_tool_call_id(self.tool_call), self.middleware_trace) + def _parse_tool_call(agent, tool_call, *, flatten_probe: bool = False) -> _ParsedCall: name = _canonical_tool_name(tool_call.function.name) @@ -605,18 +603,7 @@ def _run_with_activity_heartbeat(agent, function_name: str, fn): thread.join(timeout=2.0) -def _blocked_tool_result( - agent, - *, - function_name: str, - final_args: dict, - effective_task_id: str, - tool_call_id: str, - block_message: Optional[str], - block_error_type: str, - guardrail_decision, - trace: list, -) -> str: +def _blocked_tool_result(agent, ref: _ToolCallRef, *, block_message: Optional[str], block_error_type: str, guardrail_decision) -> str: """Synthesize the result for a call blocked by scope/plugin (``block_message``) or by guardrail policy (``guardrail_decision``) and emit its terminal post_tool_call.""" if block_message is not None: @@ -627,46 +614,32 @@ def _blocked_tool_result( result = agent._guardrail_block_result(guardrail_decision) error_type = "guardrail_block" error_message = getattr(guardrail_decision, "message", None) or "Tool blocked by guardrail policy" - _emit_terminal_post_tool_call( - agent, - function_name=function_name, - function_args=final_args, - result=result, - effective_task_id=effective_task_id, - tool_call_id=tool_call_id, - status="blocked", - error_type=error_type, - error_message=error_message, - middleware_trace=list(trace), - ) + ref.emit_post(agent, result, status="blocked", error_type=error_type, error_message=error_message) return result -def _pre_tool_block(agent, function_name: str, final_args: dict, effective_task_id: str, tool_call_id: str, trace: list): +def _pre_tool_block(agent, ref: _ToolCallRef): """Run ``pre_tool_call`` plugin hooks; returns ``(block_message, final_args)`` with any hook-modified args applied. Hook failures never block.""" try: from hermes_cli.plugins import _dispatch_pre_tool_call_hooks block_msg, modified_args = _dispatch_pre_tool_call_hooks( - function_name, - final_args, - **tool_hook_ids(agent, effective_task_id, tool_call_id), - middleware_trace=list(trace), + ref.name, + ref.args, + **tool_hook_ids(agent, ref.task_id, ref.call_id), + middleware_trace=list(ref.trace), ) - return block_msg, (final_args if modified_args is None else modified_args) + return block_msg, (ref.args if modified_args is None else modified_args) except Exception: - return None, final_args + return None, ref.args def _dispatch_authorized_once( agent, state: _ManagedToolResult, - final_args: dict[str, Any], + ref: _ToolCallRef, *, - function_name: str, - effective_task_id: str, - tool_call_id: str, execute, scope_block: str | None, display_index: int | None, @@ -675,8 +648,10 @@ def _dispatch_authorized_once( ) -> Any: """Hermes policy (scope β†’ plugin pre-hooks β†’ guardrails) then the one real dispatch. - ``begin_execution`` (concurrent start-order gate) is advanced exactly once on every - path so later-ordered workers keep moving; blocked calls advance it without a callback. + ``ref.args`` are the middleware-final args; plugin ``modify`` hooks may rewrite them + (mirrored into ``state.args``). ``begin_execution`` (concurrent start-order gate) is + advanced exactly once on every path so later-ordered workers keep moving; blocked + calls advance it without a callback. """ def _advance_start_order(callback=None) -> None: if begin_execution is not None: @@ -687,18 +662,13 @@ def _dispatch_authorized_once( block_message, block_error_type = scope_block, "tool_scope_block" if block_message is None: block_error_type = "plugin_block" - - def _resolve_pre_tool_block(): - return _pre_tool_block(agent, function_name, final_args, effective_task_id, tool_call_id, state.middleware_trace) - - block_message, final_args = ( - _resolve_pre_tool_block() if authorization_gate is None else authorization_gate.run(_resolve_pre_tool_block) - ) - state.args = final_args + resolve = lambda: _pre_tool_block(agent, ref) # noqa: E731 + block_message, ref.args = resolve() if authorization_gate is None else authorization_gate.run(resolve) + state.args = ref.args guardrail_decision = None if block_message is None: - guardrail_decision = agent._tool_guardrails.before_call(function_name, final_args) + guardrail_decision = agent._tool_guardrails.before_call(ref.name, ref.args) if guardrail_decision.allows_execution: guardrail_decision = None @@ -706,31 +676,24 @@ def _dispatch_authorized_once( _advance_start_order() state.blocked = True return _blocked_tool_result( - agent, - function_name=function_name, - final_args=final_args, - effective_task_id=effective_task_id, - tool_call_id=tool_call_id, - block_message=block_message, - block_error_type=block_error_type, - guardrail_decision=guardrail_decision, - trace=state.middleware_trace, + agent, ref, + block_message=block_message, block_error_type=block_error_type, guardrail_decision=guardrail_decision, ) - if function_name == "memory": + if ref.name == "memory": agent._turns_since_memory = 0 - elif function_name == "skill_manage": + elif ref.name == "skill_manage": agent._iters_since_skill = 0 _advance_start_order(lambda: _begin_tool_execution( agent, - function_name=function_name, - function_args=final_args, - effective_task_id=effective_task_id, - tool_call_id=tool_call_id, + function_name=ref.name, + function_args=ref.args, + effective_task_id=ref.task_id, + tool_call_id=ref.call_id, display_index=display_index, )) - return _run_with_activity_heartbeat(agent, function_name, lambda: execute(final_args)) + return _run_with_activity_heartbeat(agent, ref.name, lambda: execute(ref.args)) def _run_agent_tool_execution_middleware( @@ -768,10 +731,7 @@ def _run_agent_tool_execution_middleware( return _dispatch_authorized_once( agent, state, - final_args, - function_name=function_name, - effective_task_id=effective_task_id, - tool_call_id=tool_call_id, + _ToolCallRef(function_name, final_args, effective_task_id, tool_call_id, trace), execute=execute, scope_block=scope_block, display_index=display_index, @@ -832,38 +792,11 @@ def _resolve_sequential_tool_timeout() -> float | None: return resolve_timeout("tools.sequential_call", default=_resolve_concurrent_tool_timeout()) -def _abandoned_sequential_result( - agent, - *, - function_name: str, - function_args: dict, - effective_task_id: str, - tool_call_id: str, - middleware_trace: Optional[list], - message: str, - duration_ms: int, - status: str, - error_type: str, - error_message: str, - result_cls, -) -> _ManagedToolResult: +def _abandoned_sequential_result(agent, ref: _ToolCallRef, message: str, result_cls, **outcome) -> _ManagedToolResult: """Emit the terminal post_tool_call for a worker the sequential runner gave up on (timeout / interrupt) and wrap ``message`` in its marker ``result_cls``.""" - trace = middleware_trace if middleware_trace is not None else [] - _emit_terminal_post_tool_call( - agent, - function_name=function_name, - function_args=function_args, - result=message, - effective_task_id=effective_task_id, - tool_call_id=tool_call_id, - duration_ms=duration_ms, - status=status, - error_type=error_type, - error_message=error_message, - middleware_trace=list(trace), - ) - return _ManagedToolResult(result=result_cls(message), args=function_args, middleware_trace=trace, blocked=False, dispatched=True) + ref.emit_post(agent, message, **outcome) + return _ManagedToolResult(result=result_cls(message), args=ref.args, middleware_trace=ref.trace, blocked=False, dispatched=True) def _poll_sequential_future(agent, future, function_name: str, deadline: float | None, started: float, authorization_gate) -> tuple[str, Any]: @@ -933,12 +866,9 @@ def _run_sequential_tool_execution_middleware( worker_tid.append(tid) return _run_agent_tool_execution_middleware(agent, authorization_gate=authorization_gate, **kwargs) - ident = dict( - function_name=function_name, - function_args=function_args, - effective_task_id=effective_task_id, - tool_call_id=tool_call_id, - middleware_trace=middleware_trace, + ref = _ToolCallRef( + function_name, function_args, effective_task_id, tool_call_id, + middleware_trace if middleware_trace is not None else [], ) executor = DaemonThreadPoolExecutor(max_workers=1) future = executor.submit(propagate_context_to_thread(_run)) @@ -966,13 +896,11 @@ def _run_sequential_tool_execution_middleware( function_name, interrupt_reason, time.monotonic() - started, ) return _abandoned_sequential_result( - agent, **ident, - message=message, + agent, ref, message, _ToolCancelledResult, duration_ms=int((time.monotonic() - started) * 1000), status="cancelled", error_type="tool_interrupted", error_message=f"Tool execution cancelled: {interrupt_reason}", - result_cls=_ToolCancelledResult, ) # Only reachable when a deadline exists (interrupted returns above). @@ -983,13 +911,8 @@ def _run_sequential_tool_execution_middleware( future.cancel() _interrupt_worker_tids(agent, worker_tid) return _abandoned_sequential_result( - agent, **ident, - message=message, - duration_ms=int(timeout_s * 1000), - status="timeout", - error_type="tool_timeout", - error_message=message, - result_cls=_ToolTimeoutResult, + agent, ref, message, _ToolTimeoutResult, + duration_ms=int(timeout_s * 1000), status="timeout", error_type="tool_timeout", error_message=message, ) finally: # Never join a wedged worker. DaemonThreadPoolExecutor also keeps it out @@ -1087,12 +1010,9 @@ def _emit_tool_complete_and_risk( def _commit_tool_result( agent, messages: list, - *, - function_name: str, - function_args: dict, + ref: _ToolCallRef, function_result, - tool_call_id: str, - effective_task_id: str, + *, budget: BudgetConfig, tool_duration: float, is_error: bool, @@ -1106,6 +1026,7 @@ def _commit_tool_result( the pre-persist content for UI previews β€” or ``None`` when the flush failed (the caller must stop the batch). """ + function_name, function_args, tool_call_id, effective_task_id = ref.name, ref.args, ref.call_id, ref.task_id agent._current_tool = None _status_suffix = " (error)" if is_error else "" agent._touch_activity(f"tool completed: {function_name} ({tool_duration:.1f}s){_status_suffix}") @@ -1145,12 +1066,9 @@ def _commit_tool_result( def _observe_and_commit_tool_result( agent, messages: list, - *, - function_name: str, - function_args: dict, + ref: _ToolCallRef, function_result, - tool_call_id: str, - effective_task_id: str, + *, budget: BudgetConfig, tool_duration: float, is_error: bool, @@ -1166,6 +1084,7 @@ def _observe_and_commit_tool_result( Blocked calls never ran, so they count as neither failure nor success and are not observed. ``success_log_chars`` (sequential path) also logs the completion line. """ + function_name, function_args, tool_call_id = ref.name, ref.args, ref.call_id if not blocked: function_result = agent._append_guardrail_observation( function_name, function_args, function_result, failed=is_error, tool_call_id=tool_call_id, @@ -1186,17 +1105,8 @@ def _observe_and_commit_tool_result( logging.debug("Tool result (%d chars): %s", len(_log_result), _log_result) return _commit_tool_result( - agent, - messages, - function_name=function_name, - function_args=function_args, - function_result=function_result, - tool_call_id=tool_call_id, - effective_task_id=effective_task_id, - budget=budget, - tool_duration=tool_duration, - is_error=is_error, - blocked=blocked, + agent, messages, ref, function_result, + budget=budget, tool_duration=tool_duration, is_error=is_error, blocked=blocked, effect_disposition=effect_disposition, ) @@ -1335,7 +1245,7 @@ class _ConcurrentBatch: self.authorization_gate = _ConcurrentToolAuthorizationGate() self.timed_out_indices: set[int] = set() - def _dispatch_worker(self, index, tool_call_id, function_name, function_args, middleware_trace, scope_block, start_gate) -> Optional[_ToolOutcome]: + def _dispatch_worker(self, index: int, ref: _ToolCallRef, scope_block, start_gate: _WorkerStartOnce) -> Optional[_ToolOutcome]: """Run one call through the middleware and synthesize its slot outcome; ``None`` when the batch was abandoned at the gate (the main thread already wrote this slot, so emitting anything would double-report the tool_call_id).""" @@ -1345,70 +1255,53 @@ class _ConcurrentBatch: try: managed = _run_agent_tool_execution_middleware( agent, - function_name=function_name, - function_args=function_args, - effective_task_id=self.effective_task_id, - tool_call_id=tool_call_id, + function_name=ref.name, + function_args=ref.args, + effective_task_id=ref.task_id, + tool_call_id=ref.call_id, execute=lambda next_args: agent._invoke_tool( - function_name, + ref.name, next_args, - self.effective_task_id, - tool_call_id, + ref.task_id, + ref.call_id, messages=self.messages, pre_tool_block_checked=True, skip_tool_request_middleware=True, skip_tool_execution_middleware=True, - tool_request_middleware_trace=list(middleware_trace), + tool_request_middleware_trace=list(ref.trace), ), scope_block=scope_block, display_index=index + 1, - middleware_trace=middleware_trace, + middleware_trace=ref.trace, begin_execution=start_gate.advance, authorization_gate=self.authorization_gate, ) - result, function_args, middleware_trace = managed.result, managed.args, managed.middleware_trace + result, ref.args, ref.trace = managed.result, managed.args, managed.middleware_trace blocked, dispatched = managed.blocked, managed.dispatched except _BatchAbandoned: - logger.info("tool %s abandoned at start-order gate; skipping dispatch", function_name) + logger.info("tool %s abandoned at start-order gate; skipping dispatch", ref.name) return None except KeyboardInterrupt: try: agent.interrupt("keyboard interrupt") except Exception: pass - result = _emit_cancelled_terminal_post_tool_call( - agent, - function_name=function_name, - function_args=function_args, - effective_task_id=self.effective_task_id, - tool_call_id=tool_call_id, - start_time=start, - middleware_trace=list(middleware_trace), - ) + result = ref.emit_cancelled(agent, start) duration = time.time() - start - logger.info("tool %s cancelled (%.2fs)", function_name, duration) - return _ToolOutcome(function_name, function_args, result, duration, True, False, middleware_trace) + logger.info("tool %s cancelled (%.2fs)", ref.name, duration) + return _ToolOutcome(ref.name, ref.args, result, duration, True, False, ref.trace) except Exception as tool_error: - result = f"Error executing tool '{function_name}': {tool_error}" - logger.error("_invoke_tool raised for %s: %s", function_name, tool_error, exc_info=True) + result = f"Error executing tool '{ref.name}': {tool_error}" + logger.error("_invoke_tool raised for %s: %s", ref.name, tool_error, exc_info=True) duration = time.time() - start if not blocked and not dispatched: - _emit_terminal_post_tool_call( - agent, - function_name=function_name, - function_args=function_args, - result=result, - effective_task_id=self.effective_task_id, - tool_call_id=tool_call_id, - duration_ms=int(duration * 1000), - middleware_trace=list(middleware_trace), - ) - is_error, _ = _detect_tool_failure(function_name, result) + ref.emit_post(agent, result, duration_ms=int(duration * 1000)) + is_error, _ = _detect_tool_failure(ref.name, result) if is_error: - logger.info("tool %s failed (%.2fs): %s", function_name, duration, result[:200]) + logger.info("tool %s failed (%.2fs): %s", ref.name, duration, result[:200]) else: - logger.info("tool %s completed (%.2fs, %d chars)", function_name, duration, len(result)) - return _ToolOutcome(function_name, function_args, result, duration, is_error, blocked, middleware_trace) + logger.info("tool %s completed (%.2fs, %d chars)", ref.name, duration, len(result)) + return _ToolOutcome(ref.name, ref.args, result, duration, is_error, blocked, ref.trace) def run_worker(self, index, tool_call, function_name, function_args, middleware_trace, scope_block, start_order): """Worker function executed in a thread.""" @@ -1421,11 +1314,9 @@ class _ConcurrentBatch: # Approval/sudo callbacks and turn ContextVars are propagated by # propagate_context_to_thread() at submit. start_gate = _WorkerStartOnce(self.gate, start_order, function_name) + ref = _ToolCallRef(function_name, function_args, self.effective_task_id, _pairing_tool_call_id(tool_call), middleware_trace) try: - outcome = self._dispatch_worker( - index, _pairing_tool_call_id(tool_call), function_name, function_args, - middleware_trace, scope_block, start_gate, - ) + outcome = self._dispatch_worker(index, ref, scope_block, start_gate) if outcome is not None: self.results[index] = outcome finally: @@ -1566,15 +1457,7 @@ class _ConcurrentBatch: executor.shutdown(wait=not abandon_executor, cancel_futures=abandon_executor) -def _unfinished_tool_result( - agent, - pc: _ParsedCall, - *, - tool_call_id: str, - effective_task_id: str, - timed_out: bool, - timeout_s: float | None, -) -> tuple[str, float, Optional[str]]: +def _unfinished_tool_result(agent, ref: _ToolCallRef, *, timed_out: bool, timeout_s: float | None) -> tuple[str, float, Optional[str]]: """Synthesize the result for a slot no worker filled (deadline, interrupt, or a thread that never returned) and emit its terminal post_tool_call. @@ -1582,27 +1465,18 @@ def _unfinished_tool_result( """ if timed_out: suffix = f"{timeout_s:.1f}s" if timeout_s is not None else "the configured timeout" - function_result = f"Error executing tool '{pc.name}': timed out after {suffix}" + function_result = f"Error executing tool '{ref.name}': timed out after {suffix}" outcome = dict(duration_ms=int((timeout_s or 0.0) * 1000), status="timeout", error_type="tool_timeout", error_message=function_result) tool_duration, effect_disposition = float(timeout_s or 0.0), "unknown" elif agent._interrupt_requested: - function_result = f"[Tool execution cancelled β€” {pc.name} was skipped due to user interrupt]" + function_result = f"[Tool execution cancelled β€” {ref.name} was skipped due to user interrupt]" outcome = dict(status="cancelled", error_type="keyboard_interrupt", error_message="Tool execution cancelled by user interrupt") tool_duration, effect_disposition = 0.0, None else: - function_result = f"Error executing tool '{pc.name}': thread did not return a result" + function_result = f"Error executing tool '{ref.name}': thread did not return a result" outcome = dict(status="error", error_type="thread_missing_result", error_message=function_result) tool_duration, effect_disposition = 0.0, None - _emit_terminal_post_tool_call( - agent, - function_name=pc.name, - function_args=pc.args, - result=function_result, - effective_task_id=effective_task_id, - tool_call_id=tool_call_id, - middleware_trace=list(pc.middleware_trace), - **outcome, - ) + ref.emit_post(agent, function_result, **outcome) return function_result, tool_duration, effect_disposition @@ -1611,38 +1485,26 @@ def _append_batch_results(agent, messages: list, effective_task_id: str, batch: failed flush (the caller must stop the batch).""" for i, pc in enumerate(batch.parsed_calls): r = batch.results[i] - tool_call_id = _pairing_tool_call_id(pc.tool_call) + ref = pc.ref(effective_task_id) # A worker may finish between the deadline snapshot and this loop; # prefer its real result over a fabricated timeout. if r is None: - name, args = pc.name, pc.args + blocked = False function_result, tool_duration, effect_disposition = _unfinished_tool_result( - agent, pc, - tool_call_id=tool_call_id, - effective_task_id=effective_task_id, - timed_out=i in batch.timed_out_indices, - timeout_s=batch.timeout_s, + agent, ref, timed_out=i in batch.timed_out_indices, timeout_s=batch.timeout_s, ) committed = _commit_tool_result( - agent, messages, - function_name=name, function_args=args, function_result=function_result, - tool_call_id=tool_call_id, effective_task_id=effective_task_id, budget=budget, - tool_duration=tool_duration, is_error=True, blocked=False, + agent, messages, ref, function_result, + budget=budget, tool_duration=tool_duration, is_error=True, blocked=False, effect_disposition=effect_disposition, ) - blocked = False else: - name, args, tool_duration, blocked = r.name, r.args, r.duration, r.blocked + ref.name, ref.args, ref.trace, tool_duration, blocked = r.name, r.args, r.middleware_trace, r.duration, r.blocked if pc.parse_error is not None: - _emit_invalid_arguments_post_tool_call( - agent, function_name=name, function_args=args, result=r.result, - effective_task_id=effective_task_id, tool_call_id=tool_call_id, - ) + ref.emit_invalid_arguments(agent, r.result) committed = _observe_and_commit_tool_result( - agent, messages, - function_name=name, function_args=args, function_result=r.result, - tool_call_id=tool_call_id, effective_task_id=effective_task_id, budget=budget, - tool_duration=tool_duration, is_error=r.is_error, blocked=blocked, + agent, messages, ref, r.result, + budget=budget, tool_duration=tool_duration, is_error=r.is_error, blocked=blocked, effect_disposition="none" if blocked else None, error_preview=lambda res: _multimodal_text_summary(res)[:200], ) @@ -1651,16 +1513,16 @@ def _append_batch_results(agent, messages: list, effective_task_id: str, batch: _persisted, display_function_result, risk_metadata = committed if agent._should_emit_quiet_tool_messages(): - cute_msg = _get_cute_tool_message_impl(name, args, tool_duration, result=display_function_result) + cute_msg = _get_cute_tool_message_impl(ref.name, ref.args, tool_duration, result=display_function_result) agent._safe_print(f" {cute_msg}") elif _tool_progress_enabled(agent): _print_tool_completed(agent, i + 1, tool_duration, _multimodal_text_summary(display_function_result)) _emit_tool_complete_and_risk( agent, - function_name=name, - function_args=args, - tool_call_id=tool_call_id, + function_name=ref.name, + function_args=ref.args, + tool_call_id=ref.call_id, result=display_function_result, risk_metadata=risk_metadata, blocked=blocked, @@ -1869,25 +1731,21 @@ def _skip_remaining_sequential(agent, messages: list, remaining, effective_task_ return _append_skipped_tool_results(agent, messages, remaining, effective_task_id, **skip_kwargs) -def _append_invalid_arguments_result(agent, messages: list, pc: _ParsedCall, *, tool_call_id: str, effective_task_id: str) -> bool: +def _append_invalid_arguments_result(agent, messages: list, ref: _ToolCallRef, parse_error: str) -> bool: """Emit + append the parse-error result for a call whose arguments were not a JSON object.""" - _emit_invalid_arguments_post_tool_call( - agent, function_name=pc.name, function_args=pc.args, result=pc.parse_error, - effective_task_id=effective_task_id, tool_call_id=tool_call_id, - ) - messages.append(make_tool_result_message(pc.name, pc.parse_error, tool_call_id)) - return _flush_session_db_after_tool_progress(agent, messages, stage=f"invalid tool arguments {pc.name}") + ref.emit_invalid_arguments(agent, parse_error) + messages.append(make_tool_result_message(ref.name, parse_error, ref.call_id)) + return _flush_session_db_after_tool_progress(agent, messages, stage=f"invalid tool arguments {ref.name}") def _run_sequential_call( agent, dispatch: _SequentialDispatch, - pc: _ParsedCall, + ref: _ToolCallRef, *, + scope_block: Optional[str], messages: list, remaining_calls, - effective_task_id: str, - tool_call_id: str, display_index: int, tool_start_time: float, ) -> tuple[_ManagedToolResult, float]: @@ -1896,40 +1754,31 @@ def _run_sequential_call( KeyboardInterrupt (registry tools only) emits results for THIS and every remaining call before re-raising so the tool-call turn keeps matching results (alternation). """ - function_name, function_args, middleware_trace = pc.name, pc.args, pc.middleware_trace _spinner_result = None try: managed = _run_sequential_tool_execution_middleware( agent, - function_name=function_name, - function_args=function_args, - effective_task_id=effective_task_id, - tool_call_id=tool_call_id, + function_name=ref.name, + function_args=ref.args, + effective_task_id=ref.task_id, + tool_call_id=ref.call_id, execute=dispatch.execute, - scope_block=pc.scope_block, + scope_block=scope_block, display_index=display_index, middleware_trace=dispatch.middleware_trace_arg, ) - function_args = managed.args + ref.args = managed.args _spinner_result = managed.result except KeyboardInterrupt: if not dispatch.handles_keyboard_interrupt: raise - _spinner_result = _emit_cancelled_terminal_post_tool_call( - agent, - function_name=function_name, - function_args=function_args, - effective_task_id=effective_task_id, - tool_call_id=tool_call_id, - start_time=tool_start_time, - middleware_trace=list(middleware_trace), - ) + _spinner_result = ref.emit_cancelled(agent, tool_start_time) try: agent.interrupt("keyboard interrupt") except Exception: pass _append_skipped_tool_results( - agent, messages, remaining_calls, effective_task_id, + agent, messages, remaining_calls, ref.task_id, content="[Tool execution cancelled β€” {name} was skipped due to keyboard interrupt]", ) raise @@ -1937,57 +1786,35 @@ def _run_sequential_call( if dispatch.error_result is None: raise function_result = dispatch.error_result(tool_error) - logger.error(dispatch.error_log, function_name, tool_error, exc_info=True) - managed = _ManagedToolResult(result=function_result, args=function_args, middleware_trace=middleware_trace, blocked=False, dispatched=False) + logger.error(dispatch.error_log, ref.name, tool_error, exc_info=True) + managed = _ManagedToolResult(result=function_result, args=ref.args, middleware_trace=ref.trace, blocked=False, dispatched=False) finally: if dispatch.is_delegate: agent._delegate_spinner = None tool_duration = time.time() - tool_start_time if dispatch.finish_spinner and dispatch.finish_in_finally: - _finish_quiet_tool_spinner(agent, dispatch.spinner, function_name, function_args, tool_duration, _spinner_result) + _finish_quiet_tool_spinner(agent, dispatch.spinner, ref.name, ref.args, tool_duration, _spinner_result) if dispatch.finish_spinner and not dispatch.finish_in_finally: - _finish_quiet_tool_spinner(agent, dispatch.spinner, function_name, function_args, tool_duration, _spinner_result) + _finish_quiet_tool_spinner(agent, dispatch.spinner, ref.name, ref.args, tool_duration, _spinner_result) return managed, tool_duration -def _publish_sequential_result( - agent, - messages: list, - *, - function_name: str, - managed: _ManagedToolResult, - tool_duration: float, - index: int, - tool_call_id: str, - effective_task_id: str, - budget: BudgetConfig, -) -> bool: +def _publish_sequential_result(agent, messages: list, ref: _ToolCallRef, managed: _ManagedToolResult, *, tool_duration: float, index: int, budget: BudgetConfig) -> bool: """Terminal hook β†’ observe β†’ commit β†’ completion callbacks/print for one sequential result; False when the incremental flush failed (the caller must stop the batch).""" - function_args, function_result, middleware_trace = managed.args, managed.result, managed.middleware_trace + ref.args, ref.trace, function_result = managed.args, managed.middleware_trace, managed.result _execution_timed_out = isinstance(function_result, (_ToolTimeoutResult, _ToolCancelledResult)) # Multimodal dict results (_multimodal=True) are not sliceable as strings. _result_len = len(function_result) if isinstance(function_result, str) else len(str(function_result)) - _is_error_result, _ = _detect_tool_failure(function_name, function_result) + _is_error_result, _ = _detect_tool_failure(ref.name, function_result) # Inline-dispatched runtime tools never reach handle_function_call, so the # executor owns the one terminal post_tool_call per tool_call_id (the inner # observer is suppressed); also stops an abandoned timeout worker reporting late. if not managed.blocked and not _execution_timed_out: - _emit_terminal_post_tool_call( - agent, - function_name=function_name, - function_args=function_args, - result=function_result, - effective_task_id=effective_task_id, - tool_call_id=tool_call_id, - duration_ms=int(tool_duration * 1000), - middleware_trace=list(middleware_trace), - ) + ref.emit_post(agent, function_result, duration_ms=int(tool_duration * 1000)) committed = _observe_and_commit_tool_result( - agent, messages, - function_name=function_name, function_args=function_args, function_result=function_result, - tool_call_id=tool_call_id, effective_task_id=effective_task_id, budget=budget, - tool_duration=tool_duration, is_error=_is_error_result, blocked=managed.blocked, + agent, messages, ref, function_result, + budget=budget, tool_duration=tool_duration, is_error=_is_error_result, blocked=managed.blocked, effect_disposition="unknown" if _execution_timed_out else None, error_preview=lambda res: res[:200] if isinstance(res, str) and not agent.verbose_logging else res, success_log_chars=_result_len, @@ -1999,9 +1826,9 @@ def _publish_sequential_result( _emit_tool_complete_and_risk( agent, - function_name=function_name, - function_args=function_args, - tool_call_id=tool_call_id, + function_name=ref.name, + function_args=ref.args, + tool_call_id=ref.call_id, result=display_function_result, risk_metadata=risk_metadata, blocked=managed.blocked, @@ -2040,8 +1867,9 @@ def execute_tool_calls_sequential(agent, assistant_message, messages: list, effe break pc = _parse_tool_call(agent, tool_call, flatten_probe=True) + ref = pc.ref(effective_task_id) if pc.parse_error is not None: - if not _append_invalid_arguments_result(agent, messages, pc, tool_call_id=tool_call_id, effective_task_id=effective_task_id): + if not _append_invalid_arguments_result(agent, messages, ref, pc.parse_error): return continue @@ -2050,32 +1878,22 @@ def execute_tool_calls_sequential(agent, assistant_message, messages: list, effe # timeout policy in the callbacks. dispatch = _resolve_sequential_dispatch( agent, - function_name=pc.name, - function_args=pc.args, + function_name=ref.name, + function_args=ref.args, messages=messages, effective_task_id=effective_task_id, tool_call_id=tool_call_id, - middleware_trace=pc.middleware_trace, + middleware_trace=ref.trace, ) managed, tool_duration = _run_sequential_call( - agent, dispatch, pc, + agent, dispatch, ref, + scope_block=pc.scope_block, messages=messages, remaining_calls=tool_calls[i - 1:], - effective_task_id=effective_task_id, - tool_call_id=tool_call_id, display_index=i, tool_start_time=tool_start_time, ) - if not _publish_sequential_result( - agent, messages, - function_name=pc.name, - managed=managed, - tool_duration=tool_duration, - index=i, - tool_call_id=tool_call_id, - effective_task_id=effective_task_id, - budget=_tool_budget, - ): + if not _publish_sequential_result(agent, messages, ref, managed, tool_duration=tool_duration, index=i, budget=_tool_budget): return if agent._interrupt_requested and i < len(tool_calls): From 15750e88cebd458152a42ecc3c588cc8c0b501a4 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 2 Sep 2026 19:04:35 -0700 Subject: [PATCH 03/10] refactor(agent/tool_executor): route begin/complete/middleware kwargs through _ToolCallRef; merge batch abandon branches --- agent/tool_executor.py | 144 ++++++++++++++--------------------------- 1 file changed, 48 insertions(+), 96 deletions(-) diff --git a/agent/tool_executor.py b/agent/tool_executor.py index f4e8b90d9f..2f5bfcd1fa 100644 --- a/agent/tool_executor.py +++ b/agent/tool_executor.py @@ -247,6 +247,13 @@ class _ToolCallRef: call_id: str trace: list + def middleware_kwargs(self) -> dict[str, Any]: + """Keyword form ``_run_agent_tool_execution_middleware`` (and tests patching it) expect.""" + return { + "function_name": self.name, "function_args": self.args, "effective_task_id": self.task_id, + "tool_call_id": self.call_id, "middleware_trace": self.trace, + } + def emit_post(self, agent, result, *, trace=None, **outcome) -> None: """Emit the one terminal ``post_tool_call`` for this call (``outcome`` = status / error_type / error_message / duration_ms). Resolved through the module attribute so @@ -685,14 +692,7 @@ def _dispatch_authorized_once( elif ref.name == "skill_manage": agent._iters_since_skill = 0 - _advance_start_order(lambda: _begin_tool_execution( - agent, - function_name=ref.name, - function_args=ref.args, - effective_task_id=ref.task_id, - tool_call_id=ref.call_id, - display_index=display_index, - )) + _advance_start_order(lambda: _begin_tool_execution(agent, ref, display_index)) return _run_with_activity_heartbeat(agent, ref.name, lambda: execute(ref.args)) @@ -843,16 +843,8 @@ def _run_sequential_tool_execution_middleware( generic deadline would report ``tool_timeout`` while the prompt is still live. """ timeout_s = _resolve_sequential_tool_timeout() - kwargs = { - "function_name": function_name, - "function_args": function_args, - "effective_task_id": effective_task_id, - "tool_call_id": tool_call_id, - "execute": execute, - "scope_block": scope_block, - "display_index": display_index, - "middleware_trace": middleware_trace, - } + ref = _ToolCallRef(function_name, function_args, effective_task_id, tool_call_id, middleware_trace) + kwargs = dict(ref.middleware_kwargs(), execute=execute, scope_block=scope_block, display_index=display_index) if function_name in _NEVER_PARALLEL_TOOLS: return _run_agent_tool_execution_middleware(agent, **kwargs) @@ -866,10 +858,8 @@ def _run_sequential_tool_execution_middleware( worker_tid.append(tid) return _run_agent_tool_execution_middleware(agent, authorization_gate=authorization_gate, **kwargs) - ref = _ToolCallRef( - function_name, function_args, effective_task_id, tool_call_id, - middleware_trace if middleware_trace is not None else [], - ) + if ref.trace is None: + ref.trace = [] executor = DaemonThreadPoolExecutor(max_workers=1) future = executor.submit(propagate_context_to_thread(_run)) deadline = time.monotonic() + timeout_s if timeout_s is not None else None @@ -920,16 +910,9 @@ def _run_sequential_tool_execution_middleware( executor.shutdown(wait=not abandoned, cancel_futures=abandoned) -def _begin_tool_execution( - agent, - *, - function_name: str, - function_args: dict[str, Any], - effective_task_id: str, - tool_call_id: str, - display_index: int | None, -) -> None: +def _begin_tool_execution(agent, ref: _ToolCallRef, display_index: int | None) -> None: """Run user-visible and checkpoint preflight on final tool arguments.""" + function_name, function_args, effective_task_id, tool_call_id = ref.name, ref.args, ref.task_id, ref.call_id display_args = _redact_tool_args_for_display(function_name, function_args) or function_args if _tool_progress_enabled(agent): args_str = json.dumps(display_args, ensure_ascii=False) @@ -986,10 +969,9 @@ def _emit_tool_completed_progress(agent, function_name: str, *, duration: float, logging.debug("Tool progress callback error: %s", cb_err) -def _emit_tool_complete_and_risk( - agent, *, function_name: str, function_args: dict, tool_call_id: str, result, risk_metadata, blocked: bool -) -> None: +def _emit_tool_complete_and_risk(agent, ref: _ToolCallRef, result, risk_metadata, blocked: bool) -> None: """Fire ``tool_complete_callback`` (unless blocked) then the ``tool.output_risk`` projection.""" + function_name, function_args, tool_call_id = ref.name, ref.args, ref.call_id if not blocked and agent.tool_complete_callback: try: display_args = _redact_tool_args_for_display(function_name, function_args) or function_args @@ -1255,15 +1237,9 @@ class _ConcurrentBatch: try: managed = _run_agent_tool_execution_middleware( agent, - function_name=ref.name, - function_args=ref.args, - effective_task_id=ref.task_id, - tool_call_id=ref.call_id, + **ref.middleware_kwargs(), execute=lambda next_args: agent._invoke_tool( - ref.name, - next_args, - ref.task_id, - ref.call_id, + ref.name, next_args, ref.task_id, ref.call_id, messages=self.messages, pre_tool_block_checked=True, skip_tool_request_middleware=True, @@ -1272,7 +1248,6 @@ class _ConcurrentBatch: ), scope_block=scope_block, display_index=index + 1, - middleware_trace=ref.trace, begin_execution=start_gate.advance, authorization_gate=self.authorization_gate, ) @@ -1385,7 +1360,8 @@ class _ConcurrentBatch: if not not_done: return False - if deadline is not None and time.monotonic() >= deadline + self.authorization_gate.excluded_seconds(): + timed_out = deadline is not None and time.monotonic() >= deadline + self.authorization_gate.excluded_seconds() + if timed_out: self.timed_out_indices = {future_to_index[f] for f in not_done if f in future_to_index} logger.warning( "concurrent tool batch timed out after %.1fs; %d tool(s) still running: %s", @@ -1393,40 +1369,36 @@ class _ConcurrentBatch: len(self.timed_out_indices), ", ".join(self._running_names(not_done, future_to_index)[:5]), ) - for f in not_done: - f.cancel() - # Release gate-parked workers before interrupt fan-out so none - # later dispatches a tool just reported as timed out. - self.gate.abandon() - with agent._tool_worker_threads_lock: - worker_tids = list(agent._tool_worker_threads) - _interrupt_worker_tids(agent, worker_tids) - return True - - # Tools without interrupt checks (web_search, read_file) run to - # completion; cancel unstarted futures so we don't block on them. - if agent._interrupt_requested: + elif agent._interrupt_requested: + # Tools without interrupt checks (web_search, read_file) run to + # completion; cancel unstarted futures so we don't block on them. agent._vprint( f"{agent.log_prefix}⚑ Interrupt: cancelling {len(not_done)} pending concurrent tool(s)", force=True, ) - for f in not_done: - f.cancel() - # Release gate-parked workers so they abort instead of dispatching after - # the turn was already interrupted; then give running tools a moment to - # notice the per-thread interrupt signal and exit gracefully. - self.gate.abandon() + else: + _conc_elapsed = int(time.time() - _conc_start) + # Heartbeat every ~30s (6 Γ— 5s poll intervals) + if _conc_elapsed > 0 and _conc_elapsed % 30 < 6: + _still_running = self._running_names(not_done, future_to_index) + agent._touch_activity( + f"concurrent tools running ({_conc_elapsed}s, " + f"{len(not_done)} remaining: {', '.join(_still_running[:3])})" + ) + continue + for f in not_done: + f.cancel() + # Release gate-parked workers BEFORE interrupt fan-out so none later + # dispatches a tool the turn already reported as timed out / interrupted. + self.gate.abandon() + if timed_out: + with agent._tool_worker_threads_lock: + worker_tids = list(agent._tool_worker_threads) + _interrupt_worker_tids(agent, worker_tids) + else: + # Give running tools a moment to notice the per-thread interrupt and exit gracefully. concurrent.futures.wait(not_done, timeout=3.0) - return True - - _conc_elapsed = int(time.time() - _conc_start) - # Heartbeat every ~30s (6 Γ— 5s poll intervals) - if _conc_elapsed > 0 and _conc_elapsed % 30 < 6: - _still_running = self._running_names(not_done, future_to_index) - agent._touch_activity( - f"concurrent tools running ({_conc_elapsed}s, " - f"{len(not_done)} remaining: {', '.join(_still_running[:3])})" - ) + return True def run(self) -> None: """Dispatch the runnable calls on a daemon pool and wait for the batch.""" @@ -1518,15 +1490,7 @@ def _append_batch_results(agent, messages: list, effective_task_id: str, batch: elif _tool_progress_enabled(agent): _print_tool_completed(agent, i + 1, tool_duration, _multimodal_text_summary(display_function_result)) - _emit_tool_complete_and_risk( - agent, - function_name=ref.name, - function_args=ref.args, - tool_call_id=ref.call_id, - result=display_function_result, - risk_metadata=risk_metadata, - blocked=blocked, - ) + _emit_tool_complete_and_risk(agent, ref, display_function_result, risk_metadata, blocked) return True @@ -1758,14 +1722,10 @@ def _run_sequential_call( try: managed = _run_sequential_tool_execution_middleware( agent, - function_name=ref.name, - function_args=ref.args, - effective_task_id=ref.task_id, - tool_call_id=ref.call_id, + **dict(ref.middleware_kwargs(), middleware_trace=dispatch.middleware_trace_arg), execute=dispatch.execute, scope_block=scope_block, display_index=display_index, - middleware_trace=dispatch.middleware_trace_arg, ) ref.args = managed.args _spinner_result = managed.result @@ -1824,15 +1784,7 @@ def _publish_sequential_result(agent, messages: list, ref: _ToolCallRef, managed return False function_result, display_function_result, risk_metadata = committed - _emit_tool_complete_and_risk( - agent, - function_name=ref.name, - function_args=ref.args, - tool_call_id=ref.call_id, - result=display_function_result, - risk_metadata=risk_metadata, - blocked=managed.blocked, - ) + _emit_tool_complete_and_risk(agent, ref, display_function_result, risk_metadata, managed.blocked) if _tool_progress_enabled(agent): _print_tool_completed(agent, index, tool_duration, function_result) return True From f9743532532f1dfa447136959f112169a3e58581 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 2 Sep 2026 19:08:27 -0700 Subject: [PATCH 04/10] refactor(agent/tool_executor): _safe_callback guard replaces 5 try/except-log callback blocks --- agent/tool_executor.py | 61 +++++++++++++++++++----------------------- 1 file changed, 27 insertions(+), 34 deletions(-) diff --git a/agent/tool_executor.py b/agent/tool_executor.py index 2f5bfcd1fa..8a269f397a 100644 --- a/agent/tool_executor.py +++ b/agent/tool_executor.py @@ -910,6 +910,16 @@ def _run_sequential_tool_execution_middleware( executor.shutdown(wait=not abandoned, cancel_futures=abandoned) +def _safe_callback(callback, label: str, *args, **kwargs) -> None: + """Invoke a UI/bridge callback if set; a failing callback is logged, never fatal.""" + if not callback: + return + try: + callback(*args, **kwargs) + except Exception as callback_error: + logging.debug("%s callback error: %s", label, callback_error) + + def _begin_tool_execution(agent, ref: _ToolCallRef, display_index: int | None) -> None: """Run user-visible and checkpoint preflight on final tool arguments.""" function_name, function_args, effective_task_id, tool_call_id = ref.name, ref.args, ref.task_id, ref.call_id @@ -931,15 +941,11 @@ def _begin_tool_execution(agent, ref: _ToolCallRef, display_index: int | None) - if agent.tool_progress_callback: try: preview = _build_tool_preview(function_name, display_args) - agent.tool_progress_callback("tool.started", function_name, preview, display_args) except Exception as callback_error: logging.debug("Tool progress callback error: %s", callback_error) - - if agent.tool_start_callback: - try: - agent.tool_start_callback(tool_call_id, function_name, display_args) - except Exception as callback_error: - logging.debug("Tool start callback error: %s", callback_error) + else: + _safe_callback(agent.tool_progress_callback, "Tool progress", "tool.started", function_name, preview, display_args) + _safe_callback(agent.tool_start_callback, "Tool start", tool_call_id, function_name, display_args) if not agent._checkpoint_mgr.enabled: return @@ -955,38 +961,20 @@ def _begin_tool_execution(agent, ref: _ToolCallRef, display_index: int | None) - pass -def _emit_tool_completed_progress(agent, function_name: str, *, duration: float, is_error: bool, result) -> None: - """``tool.completed`` UI projection; downstream of the canonical append so resume - can reconstruct the result even if the UI bridge dies mid-projection.""" - if not agent.tool_progress_callback: - return - try: - agent.tool_progress_callback( - "tool.completed", function_name, None, None, - duration=duration, is_error=is_error, result=result, - ) - except Exception as cb_err: - logging.debug("Tool progress callback error: %s", cb_err) - - def _emit_tool_complete_and_risk(agent, ref: _ToolCallRef, result, risk_metadata, blocked: bool) -> None: """Fire ``tool_complete_callback`` (unless blocked) then the ``tool.output_risk`` projection.""" - function_name, function_args, tool_call_id = ref.name, ref.args, ref.call_id if not blocked and agent.tool_complete_callback: try: - display_args = _redact_tool_args_for_display(function_name, function_args) or function_args - agent.tool_complete_callback(tool_call_id, function_name, display_args, result) + display_args = _redact_tool_args_for_display(ref.name, ref.args) or ref.args except Exception as cb_err: logging.debug("Tool complete callback error: %s", cb_err) - - if risk_metadata is not None and risk_metadata.get("risk") != "low" and agent.tool_progress_callback: - try: - agent.tool_progress_callback( - "tool.output_risk", function_name, None, None, - tool_call_id=tool_call_id, risk_metadata=risk_metadata, - ) - except Exception as cb_err: - logging.debug("Tool output risk callback error: %s", cb_err) + else: + _safe_callback(agent.tool_complete_callback, "Tool complete", ref.call_id, ref.name, display_args, result) + if risk_metadata is not None and risk_metadata.get("risk") != "low": + _safe_callback( + agent.tool_progress_callback, "Tool output risk", + "tool.output_risk", ref.name, None, None, tool_call_id=ref.call_id, risk_metadata=risk_metadata, + ) def _commit_tool_result( @@ -1041,7 +1029,12 @@ def _commit_tool_result( return None if not blocked: - _emit_tool_completed_progress(agent, function_name, duration=tool_duration, is_error=is_error, result=function_result) + # ``tool.completed`` projects AFTER the canonical append + flush so resume can + # reconstruct the result even if the UI bridge dies mid-projection. + _safe_callback( + agent.tool_progress_callback, "Tool progress", + "tool.completed", function_name, None, None, duration=tool_duration, is_error=is_error, result=function_result, + ) return persisted_result, function_result, tool_message.get("_tool_output_risk") From 856d778df09c27ed6a157eca19ba540942a05395 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 2 Sep 2026 19:11:43 -0700 Subject: [PATCH 05/10] refactor(agent/tool_executor): flatten tool_search unwrap; compact docstrings/comments by hand --- agent/tool_executor.py | 378 ++++++++++++++++------------------------- 1 file changed, 142 insertions(+), 236 deletions(-) diff --git a/agent/tool_executor.py b/agent/tool_executor.py index 8a269f397a..81dc766379 100644 --- a/agent/tool_executor.py +++ b/agent/tool_executor.py @@ -1,7 +1,9 @@ """Tool-call execution: sequential and concurrent dispatch, extracted from AIAgent. -Functions take the parent ``AIAgent`` first; ``run_agent`` keeps thin wrappers, and -tests that patch ``run_agent._set_interrupt`` still work because we reach it via ``_ra()``. +Functions take the parent ``AIAgent`` first; ``run_agent`` keeps thin wrappers and is +reached lazily via ``_ra()`` so ``run_agent._set_interrupt`` patches still work. Every +call's identity travels as a ``_ToolCallRef``; both executors end in the same +observe β†’ commit β†’ project pipeline so the tool-result wire shape is produced once. """ from __future__ import annotations @@ -79,12 +81,11 @@ def _record_persisted_path_for_stub(agent, tool_call_id: str, function_result) - def _ensure_file_checkpoint(agent, function_name: str, function_args: dict, effective_task_id: str) -> None: - """Checkpoint the same workspace path that the file tool will mutate.""" + """Checkpoint the same workspace path that the file tool will mutate, resolved the way + file tools do (against the task's live cwd, which differs from the process cwd in Docker).""" file_path = function_args.get("path", "") if not file_path: return - # File tools resolve relative paths against the task's live cwd (differs from the - # process cwd in Docker); resolve the same way before locating the project root. from tools.file_tools import _resolve_path_for_task resolved_path = _resolve_path_for_task(file_path, effective_task_id or "default") @@ -93,36 +94,30 @@ 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 (default when unknown).""" + """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.""" try: ctx = getattr(getattr(agent, "context_compressor", None), "context_length", None) - # budget_for_context_window(None), not DEFAULT_BUDGET, so the MCP threshold - # override still applies when the context length isn't resolvable. return budget_for_context_window(int(ctx) if ctx else None) except Exception: return DEFAULT_BUDGET -# Maximum number of concurrent worker threads for parallel tool execution. -_MAX_TOOL_WORKERS = 8 +_MAX_TOOL_WORKERS = 8 # concurrent worker threads per batch _DEFAULT_IMAGE_PARALLEL_REQUESTS = 4 -# Generous ceiling for slow-but-valid tool work so the batch guard never preempts a legitimate attempt. +# Generous: slow-but-valid tool work must never be preempted by the batch guard. _DEFAULT_CONCURRENT_TOOL_TIMEOUT_S = 420.0 -# Start-order gate wait: long enough for an approval round-trip, short enough that one -# wedged dispatch cannot starve the batch. +# Long enough for an approval round-trip, short enough that one wedged dispatch can't starve the batch. _START_ORDER_GATE_TIMEOUT_S = 120.0 -# Fallback authorization-gate lock bound; the effective bound derives from approvals.timeout -# (see _authorization_gate_lock_timeout) since overstaying it means wedged. +# Fallback only; the effective bound derives from approvals.timeout (_authorization_gate_lock_timeout). _AUTHORIZATION_GATE_LOCK_TIMEOUT_S = 360.0 def _authorization_gate_lock_timeout() -> float: - """Bound for the authorization serialization lock: approval timeout + margin. - - Delegates to ``tools.approval.human_wait_ceiling`` so the two bounds can't drift: never - break serialization while an approval prompt is answerable, never let a wedged holder - park other workers forever. Safety-capped so a huge approvals.timeout can't overflow - Lock.acquire; deliberately NOT min()'d with the fallback so the gate never gives up early. - """ + """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.""" try: from tools.approval import human_wait_ceiling @@ -132,10 +127,8 @@ def _authorization_gate_lock_timeout() -> float: class _BatchAbandoned(BaseException): - """Raised inside a worker when the batch was abandoned before dispatch. - - BaseException so ``except Exception`` handlers in the middleware chain can't swallow it. - """ + """Raised inside a worker when the batch was abandoned before dispatch; a BaseException + so ``except Exception`` handlers in the middleware chain can't swallow it.""" def _parse_tool_arguments(raw_arguments: Any) -> tuple[dict, Optional[str]]: @@ -170,10 +163,8 @@ def _resolve_concurrent_tool_timeout() -> float | None: def _flush_session_db_after_tool_progress(agent, messages: list, *, stage: str) -> bool: - """Flush tool-call progress to the session DB before projecting it to any UI. - - Tool side effects can kill/restart the process before turn-end persistence runs. - """ + """Flush tool-call progress to the session DB before projecting it to any UI: tool side + effects can kill/restart the process before turn-end persistence runs.""" try: persisted = agent._flush_messages_to_session_db(messages) is not False if not persisted: @@ -192,8 +183,7 @@ def _flush_session_db_after_tool_progress(agent, messages: list, *, stage: str) def _image_generate_parallel_limit() -> int: - """Configured image-generation parallelism cap (conservative default; backend bursts - hit TTFB/rate-limit failures).""" + """Configured image-generation parallelism cap (conservative: backend bursts hit rate limits).""" try: from hermes_cli.config import load_config @@ -305,13 +295,10 @@ def _append_skipped_tool_results( stop_on_flush_failure: bool = True, ) -> bool: """Append one ``tool`` result per unstarted call so the assistant tool-call turn never - lacks matching results (role-alternation violation). - - ``content`` is formatted with ``{name}``. ``hook_error_type`` also emits the terminal - ``post_tool_call`` (status=cancelled) per call, ``hook_id`` overriding the hook's id. - ``flush_stage`` flushes the session DB after each append; returns False on the first - failed flush when ``stop_on_flush_failure`` (the caller must stop the batch). - """ + lacks matching results (role alternation). ``content`` is formatted with ``{name}``; + ``hook_error_type`` also emits the terminal ``post_tool_call`` (status=cancelled) per + call with ``hook_id`` overriding the hook's id; ``flush_stage`` flushes after each + append and returns False on the first failed flush when ``stop_on_flush_failure``.""" for tc in tool_calls: name = _tc_name(tc) result = content.format(name=name) @@ -329,12 +316,9 @@ def _append_skipped_tool_results( def _tool_search_scoped_names(agent) -> frozenset: - """Deferrable tool names the session may invoke via ``tool_call``. - - The Tool Search unwrap bypasses the bridge's scope check in - ``model_tools.handle_function_call``, so restricted sessions are validated against - this set. Cached on the agent; refreshed when the registry generation changes. - """ + """Deferrable tool names the session may invoke via ``tool_call``; the unwrap bypasses + the bridge's scope check in ``model_tools.handle_function_call``, so restricted sessions + validate against this set. Cached on the agent, keyed by registry scope/generation.""" try: import model_tools from tools import tool_search as _ts @@ -354,13 +338,9 @@ def _tool_search_scoped_names(agent) -> frozenset: if cached is not None and cached[0] == cache_key: return cached[1] try: - scoped_defs = model_tools.get_tool_definitions( - enabled_toolsets=enabled, - disabled_toolsets=disabled, - quiet_mode=True, - skip_tool_search_assembly=True, - ) or [] - names = _ts.scoped_deferrable_names(scoped_defs) + names = _ts.scoped_deferrable_names(model_tools.get_tool_definitions( + enabled_toolsets=enabled, disabled_toolsets=disabled, quiet_mode=True, skip_tool_search_assembly=True, + ) or []) except Exception: names = frozenset() try: @@ -380,44 +360,40 @@ def _canonical_tool_name(function_name: str) -> str: def _unwrap_tool_search_call( agent, function_name: str, function_args: dict, *, flatten_probe: bool = False ) -> tuple[str, dict, Optional[str]]: - """Peel the ``tool_call`` bridge so downstream hooks see the underlying tool. + """Peel the ``tool_call`` bridge so downstream hooks (checkpointing, guardrails, plugin + hooks, activity feed) see the underlying tool; ``tool_call.function`` stays untouched for + the transcript and tool_call_id pairing. - Checkpointing, guardrails, plugin hooks and the activity feed must observe the real - tool name, not the bridge. ``tool_call.function`` stays untouched for the transcript - and tool_call_id pairing. The unwrap bypasses handle_function_call's scope check, so - session toolset scope is enforced HERE. Returns ``(name, args, scope_block)``; - ``scope_block`` is the block message when the underlying tool is out of scope or its - args fail the deferred-schema probe (``flatten_probe`` collapses the probe's JSON - payload to one plain string for callers that wrap the message in ``{"error": ...}``). + The unwrap bypasses handle_function_call's scope check, so session toolset scope is + enforced HERE. Returns ``(name, args, scope_block)``; ``scope_block`` is the block + message when the underlying tool is out of scope or its args fail the deferred-schema + probe (``flatten_probe`` collapses the probe's JSON payload to one plain string for + callers that wrap the message in ``{"error": ...}``). """ scope_block: Optional[str] = None try: from tools import tool_search as _ts - if function_name == _ts.TOOL_CALL_NAME: - underlying, underlying_args, err = _ts.resolve_underlying_call(function_args) - if not err and underlying: - if underlying in _tool_search_scoped_names(agent): - # Validate before unwrapping: the generic bridge hides the concrete - # parameter schema from provider-native tool-call validation. - probe_err = _ts.validate_deferred_call_args(underlying, underlying_args) - if probe_err is None: - return underlying, underlying_args, None - scope_block = probe_err - if flatten_probe: - try: - probe = json.loads(probe_err) - scope_block = ( - f"{probe.get('error', '')} Parameters schema: " - f"{json.dumps(probe.get('parameters', {}), ensure_ascii=False)}. " - f"{probe.get('hint', '')}" - ).strip() - except Exception: - scope_block = probe_err - else: - scope_block = ( - f"'{underlying}' is not available in this session. " - "Use tool_search to find tools you can call." - ) + if function_name != _ts.TOOL_CALL_NAME: + return function_name, function_args, None + underlying, underlying_args, err = _ts.resolve_underlying_call(function_args) + if err or not underlying: + return function_name, function_args, None + if underlying not in _tool_search_scoped_names(agent): + return function_name, function_args, ( + f"'{underlying}' is not available in this session. Use tool_search to find tools you can call." + ) + # Validate before unwrapping: the generic bridge hides the concrete + # parameter schema from provider-native tool-call validation. + scope_block = _ts.validate_deferred_call_args(underlying, underlying_args) + if scope_block is None: + return underlying, underlying_args, None + if flatten_probe: + probe = json.loads(scope_block) + scope_block = ( + f"{probe.get('error', '')} Parameters schema: " + f"{json.dumps(probe.get('parameters', {}), ensure_ascii=False)}. " + f"{probe.get('hint', '')}" + ).strip() except Exception: pass return function_name, function_args, scope_block @@ -461,22 +437,17 @@ class _ToolTimeoutResult(str): class _ToolCancelledResult(str): - """Marker for a synthesized sequential-tool user-interrupt result. - - The terminal post_tool_call event was already emitted (status=cancelled), so a - late-finishing abandoned worker must not report success. - """ + """Marker for a synthesized sequential-tool user-interrupt result; its terminal + post_tool_call was already emitted, so a late-finishing abandoned worker must not report.""" class _ConcurrentToolAuthorizationGate: """Serialize policy prompts and exclude human approval waits from batch deadlines. - The acquire is BOUNDED: on expiry the worker prompts unserialized rather than - starving the batch behind a wedged plugin/approval client. - - Deadline 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. + The acquire is BOUNDED: on expiry the worker prompts unserialized rather than starving + 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. """ def __init__(self, *, lock_timeout: float | None = None, session_key: str | None = None) -> None: @@ -487,8 +458,8 @@ class _ConcurrentToolAuthorizationGate: try: from tools.approval import get_current_session_key - # Snapshot on the SUBMITTING thread: excluded_seconds() is polled - # from the batch wait loop, whose context may differ from workers'. + # Snapshot on the SUBMITTING thread: excluded_seconds() is polled from the + # batch wait loop, whose context may differ from the workers'. self._session_key = get_current_session_key() except Exception: logger.debug( @@ -528,11 +499,8 @@ class _ConcurrentToolAuthorizationGate: @contextlib.contextmanager def _registered_tool_worker(agent): - """Track this worker tid for interrupt fan-out (``AIAgent.interrupt()``). - - On exit (including BaseException, which bypasses ``except Exception``) the tid is - discarded and its interrupt bit cleared so a recycled tid starts clean. - """ + """Track this worker tid for interrupt fan-out (``AIAgent.interrupt()``); on ANY exit + (incl. BaseException) discard it and clear its interrupt bit so a recycled tid starts clean.""" tid = threading.current_thread().ident with agent._tool_worker_threads_lock: agent._tool_worker_threads.add(tid) @@ -570,8 +538,7 @@ def _set_worker_activity_callback(agent) -> None: pass -# Heartbeat cadence; must stay far below the gateway turn-inactivity timeout -# (default 1800s) so a silent-but-healthy tool never looks idle. +# Must stay far below the gateway turn-inactivity timeout (default 1800s) so a silent tool never looks idle. _TOOL_ACTIVITY_HEARTBEAT_INTERVAL_S = 30.0 @@ -588,8 +555,7 @@ def _run_tool_activity_heartbeat( while not stop_event.wait(interval): agent._touch_activity(label) except Exception: - # A heartbeat must never break the agent loop. - pass + pass # a heartbeat must never break the agent loop def _run_with_activity_heartbeat(agent, function_name: str, fn): @@ -655,10 +621,9 @@ def _dispatch_authorized_once( ) -> Any: """Hermes policy (scope β†’ plugin pre-hooks β†’ guardrails) then the one real dispatch. - ``ref.args`` are the middleware-final args; plugin ``modify`` hooks may rewrite them - (mirrored into ``state.args``). ``begin_execution`` (concurrent start-order gate) is - advanced exactly once on every path so later-ordered workers keep moving; blocked - calls advance it without a callback. + Plugin ``modify`` hooks may rewrite ``ref.args`` (mirrored into ``state.args``). + ``begin_execution`` (concurrent start-order gate) is advanced exactly once on every + path so later-ordered workers keep moving; blocked calls advance it without a callback. """ def _advance_start_order(callback=None) -> None: if begin_execution is not None: @@ -772,21 +737,15 @@ def _run_agent_tool_execution_middleware( return state -# Sequential wait-loop interrupt poll cadence: /stop lands within ~1s even when -# the tool never polls is_interrupted(). +# Sequential wait-loop poll cadence: /stop lands within ~1s even if the tool never polls is_interrupted(). _SEQUENTIAL_INTERRUPT_POLL_SECONDS = 1.0 def _resolve_sequential_tool_timeout() -> float | None: - """Deadline for one sequential tool call. - - ``timeouts.tools.sequential_call`` wins; unset inherits the concurrent batch deadline - so the two paths can't drift. ``0``/negative disables the bound. - - Deliberately NOT ``agent.deadline.run_bounded_sync``: both executors extend their - deadline while an approval prompt is open (MUST-preserve), which the fixed-deadline - primitive can't express. - """ + """Deadline for one sequential call: ``timeouts.tools.sequential_call``, else the + concurrent batch deadline so the two paths can't drift; ``0``/negative disables. + Deliberately NOT ``agent.deadline.run_bounded_sync``: both executors extend the + deadline while an approval prompt is open, which a fixed deadline can't express.""" from agent.deadline import resolve_timeout return resolve_timeout("tools.sequential_call", default=_resolve_concurrent_tool_timeout()) @@ -800,12 +759,10 @@ def _abandoned_sequential_result(agent, ref: _ToolCallRef, message: str, result_ def _poll_sequential_future(agent, future, function_name: str, deadline: float | None, started: float, authorization_gate) -> tuple[str, Any]: - """Wait for the worker in interrupt-poll slices, extending the deadline by human - approval wait. Returns ``("done", result)``, ``("timeout", None)`` or ``("interrupted", None)``. - + """Wait for the worker in interrupt-poll slices, extending the deadline by human approval + wait; returns ``("done", result)``, ``("timeout", None)`` or ``("interrupted", None)``. A disabled deadline still polls: this loop is what makes a non-cooperative tool - interruptible, so no deadline must not mean no interrupt checks. - """ + interruptible, so no deadline must not mean no interrupt checks.""" _last_heartbeat = 0 while True: wait_slice = _SEQUENTIAL_INTERRUPT_POLL_SECONDS @@ -838,10 +795,8 @@ def _run_sequential_tool_execution_middleware( middleware_trace: list[dict[str, Any]] | None = None, ) -> _ManagedToolResult: """Run one sequential call on a worker thread under the concurrent executor's deadline. - Interactive tools (``clarify``) own their wait via ``agent.clarify_timeout``; the - generic deadline would report ``tool_timeout`` while the prompt is still live. - """ + generic deadline would report ``tool_timeout`` while the prompt is still live.""" timeout_s = _resolve_sequential_tool_timeout() ref = _ToolCallRef(function_name, function_args, effective_task_id, tool_call_id, middleware_trace) kwargs = dict(ref.middleware_kwargs(), execute=execute, scope_block=scope_block, display_index=display_index) @@ -870,14 +825,13 @@ def _run_sequential_tool_execution_middleware( if state == "done": return result if state == "interrupted": - # Belt-and-braces: interrupt() already fans out to tracked worker - # tids, but the worker may have registered after the fan-out ran. + # interrupt() already fanned out to tracked tids, but this worker may have + # registered after that ran; then 3s grace (mirrors the concurrent path). _interrupt_worker_tids(agent, worker_tid, reason=getattr(agent, "_tool_interrupt_reason", None)) - # Grace for a cooperative tool to notice its interrupt bit (mirrors the concurrent path's 3s). concurrent.futures.wait([future], timeout=3.0) if future.done() and not future.cancelled(): return future.result() - abandoned = True # reuse the abandon-shutdown path in finally + abandoned = True future.cancel() interrupt_reason = getattr(agent, "_tool_interrupt_reason", None) or "interrupt requested" message = f"[Tool execution cancelled β€” {function_name} was abandoned: {interrupt_reason}]" @@ -905,8 +859,7 @@ def _run_sequential_tool_execution_middleware( duration_ms=int(timeout_s * 1000), status="timeout", error_type="tool_timeout", error_message=message, ) finally: - # Never join a wedged worker. DaemonThreadPoolExecutor also keeps it out - # of the stdlib atexit join, matching the concurrent timeout path. + # Never join a wedged worker (daemon pool also keeps it out of the atexit join). executor.shutdown(wait=not abandoned, cancel_futures=abandoned) @@ -989,13 +942,10 @@ def _commit_tool_result( blocked: bool, effect_disposition, ): - """Mark the tool done; persist/spill, hint, wrap and append its result; flush the - session DB; then project ``tool.completed``. - - Returns ``(persisted_result, display_result, risk_metadata)`` β€” ``display_result`` is - the pre-persist content for UI previews β€” or ``None`` when the flush failed (the - caller must stop the batch). - """ + """Mark the tool done; persist/spill, hint, wrap and append its result; flush the session + DB; then project ``tool.completed``. Returns ``(persisted_result, display_result, + risk_metadata)`` (``display_result`` = pre-persist content for UI previews) or ``None`` + when the flush failed (the caller must stop the batch).""" function_name, function_args, tool_call_id, effective_task_id = ref.name, ref.args, ref.call_id, ref.task_id agent._current_tool = None _status_suffix = " (error)" if is_error else "" @@ -1020,8 +970,8 @@ def _commit_tool_result( else: persisted_result += subdir_hints - # Unwrap _multimodal dicts to an OpenAI-style content list; text-only servers - # get a string-safe fallback so a rejected image result never poisons history. + # Multimodal dicts become an OpenAI-style content list; text-only servers get a + # string-safe fallback so a rejected image result never poisons history. _tool_content = agent._tool_result_content_for_active_model(function_name, persisted_result) tool_message = make_tool_result_message(function_name, _tool_content, tool_call_id, effect_disposition=effect_disposition) messages.append(tool_message) @@ -1054,11 +1004,9 @@ def _observe_and_commit_tool_result( verbose_text: Callable[[Any], Any] = lambda result: result, ): """Guardrail-observe a result that actually ran, log its outcome, feed the turn-end - file-mutation verifier, then ``_commit_tool_result`` it. - - Blocked calls never ran, so they count as neither failure nor success and are not - observed. ``success_log_chars`` (sequential path) also logs the completion line. - """ + file-mutation verifier, then ``_commit_tool_result`` it. Blocked calls never ran, so + they count as neither failure nor success and are not observed; ``success_log_chars`` + (sequential path) also logs the completion line.""" function_name, function_args, tool_call_id = ref.name, ref.args, ref.call_id if not blocked: function_result = agent._append_guardrail_observation( @@ -1087,11 +1035,8 @@ def _observe_and_commit_tool_result( def _finalize_tool_batch(agent, messages: list, effective_task_id: str, num_tools: int, budget: BudgetConfig) -> None: - """Per-turn aggregate budget enforcement, then /steer injection. - - /steer stays pending until AFTER budget enforcement so the steer marker is never - truncated or discarded when enforcement replaces a tool result; see ``steer()``. - """ + """Per-turn aggregate budget enforcement, then /steer injection β€” in that order, so the + steer marker is never truncated/discarded when enforcement replaces a result.""" if num_tools <= 0: return enforce_turn_budget(messages[-num_tools:], env=get_active_env(effective_task_id), config=budget) @@ -1138,11 +1083,8 @@ def _start_order_gate_timeout(batch_timeout: float | None) -> float: class _StartOrderGate: - """Serialize worker dispatch by submit order (prompts appear in call order). - - ``abandon()`` releases every parked worker so none dispatches a tool the turn - already reported as timed out / interrupted. - """ + """Serialize worker dispatch by submit order (prompts appear in call order); ``abandon()`` + releases every parked worker so none dispatches a tool the turn already gave up on.""" def __init__(self, timeout: float) -> None: self._condition = threading.Condition() @@ -1158,17 +1100,14 @@ class _StartOrderGate: def begin_in_order(self, order: int, callback=None, *, tool_name: str = "") -> bool: """Wait for ``order``, run ``callback``, advance. Returns False if abandoned.""" with self._condition: - # Bounded wait so one wedged dispatch can't starve/leak later-ordered workers; - # on expiry proceed out of order (interleaved prompts beat starvation). - # >= (not ==) releases every skipped worker at once; abandoned short-circuits. + # Bounded wait so one wedged dispatch can't starve later-ordered workers; on + # expiry proceed out of order (interleaved prompts beat starvation). ``>=`` (not + # ``==``) releases every skipped worker at once; abandoned short-circuits. in_order = self._condition.wait_for( - lambda: self._next_order >= order or self.abandoned.is_set(), - timeout=self._timeout, + lambda: self._next_order >= order or self.abandoned.is_set(), timeout=self._timeout, ) if self.abandoned.is_set(): - # Do not run the callback or advance the counter: the turn has - # already synthesized this tool's result and moved on. - return False + return False # the turn already synthesized this result; don't advance if not in_order: logger.warning( "start-order gate timed out for %s (order=%d next=%d); proceeding out of order", @@ -1221,9 +1160,9 @@ class _ConcurrentBatch: self.timed_out_indices: set[int] = set() def _dispatch_worker(self, index: int, ref: _ToolCallRef, scope_block, start_gate: _WorkerStartOnce) -> Optional[_ToolOutcome]: - """Run one call through the middleware and synthesize its slot outcome; ``None`` - when the batch was abandoned at the gate (the main thread already wrote this slot, - so emitting anything would double-report the tool_call_id).""" + """Run one call through the middleware and synthesize its slot outcome; ``None`` when + abandoned at the gate (the main thread already wrote this slot; emitting would + double-report the tool_call_id).""" agent = self.agent start = time.time() blocked = dispatched = False @@ -1275,12 +1214,10 @@ class _ConcurrentBatch: """Worker function executed in a thread.""" agent = self.agent with _registered_tool_worker(agent) as _worker_tid: - # Race: interrupt may have fanned out before our registration; apply it to our own tid now. + # An interrupt may have fanned out before our registration; apply it to our tid. if agent._interrupt_requested: _interrupt_worker_tids(agent, [_worker_tid], reason=getattr(agent, "_tool_interrupt_reason", None)) _set_worker_activity_callback(agent) - # Approval/sudo callbacks and turn ContextVars are propagated by - # propagate_context_to_thread() at submit. start_gate = _WorkerStartOnce(self.gate, start_order, function_name) ref = _ToolCallRef(function_name, function_args, self.effective_task_id, _pairing_tool_call_id(tool_call), middleware_trace) try: @@ -1288,19 +1225,16 @@ class _ConcurrentBatch: if outcome is not None: self.results[index] = outcome finally: - # Teardown advance keeps later-ordered workers moving; never let the - # abandonment signal escape here. with contextlib.suppress(_BatchAbandoned): - start_gate.advance() + start_gate.advance() # keep later-ordered workers moving def submit_all(self, executor, runnable_calls) -> tuple[list, dict]: """Submit every runnable call; on interpreter shutdown, synthesize error results - for the unsubmitted remainder instead of raising.""" + for the unsubmitted remainder instead of raising. ``propagate_context_to_thread`` + carries turn ContextVars and thread-local approval/sudo callbacks into the worker.""" futures = [] future_to_index = {} for submit_index, (i, tc, name, args, scope_block) in enumerate(runnable_calls): - # Propagate turn ContextVars and thread-local approval/sudo - # callbacks into the worker; clears callbacks on exit. try: f = executor.submit( propagate_context_to_thread(self.run_worker), @@ -1333,9 +1267,8 @@ class _ConcurrentBatch: return [self.parsed_calls[future_to_index[f]].name for f in not_done if f in future_to_index] def await_completion(self, futures, future_to_index, deadline: float | None) -> bool: - """Wait with periodic heartbeats (gateway inactivity monitor) and interrupt checks - (/stop or a new message). Returns True when the batch was abandoned (deadline or - interrupt) and the executor must not join its workers.""" + """Wait with periodic heartbeats and interrupt checks; True when the batch was + abandoned (deadline or interrupt) and the executor must not join its workers.""" agent = self.agent _conc_start = time.time() while True: @@ -1404,8 +1337,7 @@ class _ConcurrentBatch: return deadline = time.monotonic() + self.timeout_s if self.timeout_s is not None else None max_workers = _max_workers_for_tool_batch(runnable_calls) - # Daemon workers: stdlib ThreadPoolExecutor's atexit join would let one - # wedged tool thread block interpreter exit forever. + # Daemon workers: the stdlib pool's atexit join would let one wedged tool block exit. from tools.daemon_pool import DaemonThreadPoolExecutor executor = DaemonThreadPoolExecutor(max_workers=max_workers) abandon_executor = False @@ -1413,21 +1345,17 @@ class _ConcurrentBatch: futures, future_to_index = self.submit_all(executor, runnable_calls) abandon_executor = self.await_completion(futures, future_to_index, deadline) finally: - # Any abandoning exit from the wait loop (including the exception - # path) must release gate-parked workers. + # Every abandoning exit releases gate-parked workers and leaves wedged threads + # detached rather than joining them; normal completion joins. if abandon_executor: self.gate.abandon() - # On abandon do NOT join hung workers: a wedged thread is left detached - # rather than deadlocking the batch. Normal completion joins. executor.shutdown(wait=not abandon_executor, cancel_futures=abandon_executor) def _unfinished_tool_result(agent, ref: _ToolCallRef, *, timed_out: bool, timeout_s: float | None) -> tuple[str, float, Optional[str]]: - """Synthesize the result for a slot no worker filled (deadline, interrupt, or a - thread that never returned) and emit its terminal post_tool_call. - - Returns ``(function_result, tool_duration, effect_disposition)``. - """ + """Synthesize the result for a slot no worker filled (deadline, interrupt, or a thread + that never returned), emit its terminal post_tool_call, and return + ``(function_result, tool_duration, effect_disposition)``.""" if timed_out: suffix = f"{timeout_s:.1f}s" if timeout_s is not None else "the configured timeout" function_result = f"Error executing tool '{ref.name}': timed out after {suffix}" @@ -1489,15 +1417,11 @@ def _append_batch_results(agent, messages: list, effective_task_id: str, batch: def execute_tool_calls_concurrent(agent, assistant_message, messages: list, effective_task_id: str, api_call_count: int = 0, *, finalize: bool = True) -> None: """Execute tool calls concurrently; results are appended in original call order. - ``finalize=False`` skips end-of-batch budget enforcement and /steer injection (the - segmented dispatcher owns turn-end work). - """ + segmented dispatcher owns turn-end work).""" tool_calls = assistant_message.tool_calls num_tools = len(tool_calls) - - # Resolve the context-scaled tool-output budget once per turn, not per result. - _tool_budget = _budget_for_agent(agent) + _tool_budget = _budget_for_agent(agent) # once per turn, not per result if agent._interrupt_requested: print(f"{agent.log_prefix}⚑ Interrupt: skipping {num_tools} tool call(s)") @@ -1516,12 +1440,9 @@ def execute_tool_calls_concurrent(agent, assistant_message, messages: list, effe if _tool_progress_enabled(agent): print(f" ⚑ Concurrent: {num_tools} tool calls β€” {tool_names_str}") - # Resolved before the batch is built so the start-order gate can clamp - # its own bound against the batch deadline it must stay under. + # Resolved before the batch is built so the start-order gate can clamp under the deadline. timeout_s = _resolve_concurrent_tool_timeout() batch = _ConcurrentBatch(agent, messages, effective_task_id, parsed_calls, timeout_s) - - # Touch activity before launching workers so the gateway knows we're executing tools. agent._current_tool = tool_names_str agent._touch_activity(f"executing {num_tools} tools concurrently: {tool_names_str}") @@ -1544,10 +1465,8 @@ def execute_tool_calls_concurrent(agent, assistant_message, messages: list, effe def _start_quiet_tool_spinner(agent, function_name: str, function_args: dict, *, gate: bool = True, label: Optional[str] = None): - """Start the quiet-mode kawaii spinner for one tool call, or return None. - - ``gate=False`` skips ``_should_start_quiet_spinner`` (context-engine tools always spin). - """ + """Start the quiet-mode kawaii spinner for one tool call, or return None; ``gate=False`` + skips ``_should_start_quiet_spinner`` (context-engine tools always spin).""" if not agent._should_emit_quiet_tool_messages(): return None if gate and not agent._should_start_quiet_spinner(): @@ -1609,11 +1528,9 @@ def _resolve_sequential_dispatch( tool_call_id: str, middleware_trace: list, ) -> _SequentialDispatch: - """Pick the execute callable for one sequential call and start its spinner. - - Precedence is the historical if/elif order: inline agent-level tools, delegate_task, - context-engine tools, memory-provider tools, then the registry. - """ + """Pick the execute callable for one sequential call and start its spinner. Precedence: + inline agent-level tools, delegate_task, context-engine tools, memory-provider tools, + then the registry.""" if function_name != "delegate_task" and function_name in INLINE_TOOL_EXECUTORS: # Agent-level tools that need live AIAgent state; table shared with invoke_tool. inline_executor = INLINE_TOOL_EXECUTORS[function_name] @@ -1707,10 +1624,8 @@ def _run_sequential_call( tool_start_time: float, ) -> tuple[_ManagedToolResult, float]: """Run one sequential call with its spinner/error policy; returns ``(managed, duration)``. - - KeyboardInterrupt (registry tools only) emits results for THIS and every remaining - call before re-raising so the tool-call turn keeps matching results (alternation). - """ + KeyboardInterrupt (registry tools only) emits results for THIS and every remaining call + before re-raising so the tool-call turn keeps matching results (alternation).""" _spinner_result = None try: managed = _run_sequential_tool_execution_middleware( @@ -1784,21 +1699,17 @@ def _publish_sequential_result(agent, messages: list, ref: _ToolCallRef, managed def execute_tool_calls_sequential(agent, assistant_message, messages: list, effective_task_id: str, api_call_count: int = 0, *, finalize: bool = True) -> None: - """Execute tool calls sequentially (single calls or interactive tools). - - ``finalize=False`` skips end-of-batch budget enforcement and /steer injection (the - segmented dispatcher owns turn-end work). - """ - # Resolve the context-scaled tool-output budget once per turn, not per result. - _tool_budget = _budget_for_agent(agent) + """Execute tool calls sequentially (single calls or interactive tools). ``finalize=False`` + skips end-of-batch budget enforcement and /steer injection (the segmented dispatcher + owns turn-end work).""" + _tool_budget = _budget_for_agent(agent) # once per turn, not per result tool_calls = assistant_message.tool_calls for i, tool_call in enumerate(tool_calls, 1): tool_call_id = _pairing_tool_call_id(tool_call) if getattr(agent, "_incremental_persistence_failed", False): return - # SAFETY: check interrupt BEFORE each tool so a "stop" during the previous - # tool skips all remaining ones. + # Check interrupt BEFORE each tool so a "stop" during the previous one skips the rest. if agent._interrupt_requested: if not _skip_remaining_sequential( agent, messages, tool_calls[i - 1:], effective_task_id, @@ -1819,8 +1730,6 @@ def execute_tool_calls_sequential(agent, assistant_message, messages: list, effe continue tool_start_time = time.time() - # One bounded execution funnel for every runtime-tool branch; no duplicated - # timeout policy in the callbacks. dispatch = _resolve_sequential_dispatch( agent, function_name=ref.name, @@ -1856,14 +1765,11 @@ def execute_tool_calls_sequential(agent, assistant_message, messages: list, effe def execute_tool_calls_segmented(agent, assistant_message, messages: list, effective_task_id: str, api_call_count: int = 0, segments=None) -> None: - """Execute a mixed batch as ordered parallel/sequential segments. - - ``segments`` is the ``(kind, calls)`` plan from ``_plan_tool_batch_segments``; - contiguous segments preserve per-call result order and barrier boundaries exactly - as fully-sequential execution. Turn-end work (budget + /steer) runs once here; - segment executors run with ``finalize=False``. Each segment executor checks the - interrupt flag up front, so an interrupt drains later segments with one result per call. - """ + """Execute a mixed batch as ordered parallel/sequential segments (the ``(kind, calls)`` + plan from ``_plan_tool_batch_segments``), preserving per-call result order and barrier + boundaries exactly as fully-sequential execution. Turn-end work (budget + /steer) runs + once here (segments run with ``finalize=False``); each segment executor checks the + interrupt flag up front, so an interrupt drains later segments with one result per call.""" from types import SimpleNamespace if segments is None: From 01bf2ca4b3d732140337e624054f2ff7a14f80fc Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 2 Sep 2026 19:14:55 -0700 Subject: [PATCH 06/10] refactor(agent/tool_dispatch_helpers): flatten bridge peel / overlap / risk helpers; compact docstrings --- agent/tool_dispatch_helpers.py | 184 ++++++++++++--------------------- 1 file changed, 64 insertions(+), 120 deletions(-) diff --git a/agent/tool_dispatch_helpers.py b/agent/tool_dispatch_helpers.py index 54bedab1ae..fd56cb0477 100644 --- a/agent/tool_dispatch_helpers.py +++ b/agent/tool_dispatch_helpers.py @@ -1,12 +1,10 @@ """Tool-dispatch helpers β€” parallelism gating, multimodal envelopes, mutation tracking. -Stateless module-level utilities extracted from ``run_agent.py``, which -re-exports each name so existing ``from run_agent import ...`` imports keep -working. Groups: batch-parallelism planner (path-overlap admission; V4A patch -scope comes from patch-body headers, not a decoy ``path=``), multimodal -``{"_multimodal": True, "content": [...], "text_summary": ...}`` envelope -helpers, per-turn file-mutation verifier inputs, trajectory normalisation, and -the tool-result message constructor with its untrusted-content wrapping. +Stateless utilities extracted from ``run_agent.py`` (which re-exports each name): the +batch-parallelism planner (path-overlap admission; V4A patch scope comes from patch-body +headers, not a decoy ``path=``), multimodal ``{"_multimodal": True, "content": [...], +"text_summary": ...}`` envelope helpers, file-mutation verifier inputs, trajectory +normalisation, and the tool-result message constructor with untrusted-content wrapping. """ from __future__ import annotations @@ -45,15 +43,13 @@ _PARALLEL_SAFE_TOOLS = frozenset({ "web_search", }) -# Filesystem tools admitted by path overlap. Readers may share a subtree; a -# writer conflicts with ANY overlapping reservation. This keeps a batched -# read_file/search_files from observing pre-mutation state when the model -# batches it alongside the patch/write_file it depends on. +# Filesystem tools admitted by path overlap: readers may share a subtree, a writer conflicts +# with ANY overlapping reservation (so a batched read never observes pre-mutation state). _PATH_SCOPED_READERS = frozenset({"read_file", "search_files"}) _PATH_SCOPED_WRITERS = frozenset({"write_file", "patch"}) _PATH_SCOPED_TOOLS = _PATH_SCOPED_READERS | _PATH_SCOPED_WRITERS -# Patterns that indicate a terminal command may modify/delete files. +# Terminal commands that may modify/delete files. _DESTRUCTIVE_PATTERNS = re.compile( r"""(?:^|\s|&&|\|\||;|`)(?: rm\s|rmdir\s| @@ -90,23 +86,18 @@ _PARALLEL_SAFE_BRIDGE_LOOKUPS = frozenset({"tool_search", "tool_describe"}) def _peel_bridge_call(tool_name: str, function_args: dict) -> tuple[str, dict]: - """Resolve a ``tool_call`` bridge invocation to ``(underlying_name, underlying_args)``. - - With tool search active the model emits the literal name ``tool_call`` for - every deferred tool, so admission must be decided on the underlying tool - (as the executors' unwrap does). An unparseable bridge call is returned - unchanged: it stays a sequential barrier and fails at dispatch as before. - """ + """Resolve a ``tool_call`` bridge invocation to ``(underlying_name, underlying_args)`` so + admission is decided on the real tool (as the executors' unwrap does). An unparseable + bridge call is returned unchanged: it stays a sequential barrier and fails at dispatch.""" try: from tools.tool_search import TOOL_CALL_NAME, resolve_underlying_call - if tool_name != TOOL_CALL_NAME: - return tool_name, function_args - underlying, underlying_args, err = resolve_underlying_call(function_args) - if err is not None or not underlying: - return tool_name, function_args - return underlying, underlying_args + if tool_name == TOOL_CALL_NAME: + underlying, underlying_args, err = resolve_underlying_call(function_args) + if err is None and underlying: + return underlying, underlying_args except Exception: - return tool_name, function_args + pass + return tool_name, function_args def _batch_admission(tool_call, execution_cwd: Optional[Path]) -> tuple[str, List[Path], bool] | None: @@ -121,16 +112,11 @@ def _batch_admission(tool_call, execution_cwd: Optional[Path]) -> tuple[str, Lis _raw = tool_call.function.arguments logging.debug( "Could not parse args for %s β€” treating as sequential barrier; raw=%s", - tool_name, - _raw[:200] if isinstance(_raw, str) else repr(_raw)[:200], + tool_name, _raw[:200] if isinstance(_raw, str) else repr(_raw)[:200], ) return None if not isinstance(function_args, dict): - logging.debug( - "Non-dict args for %s (%s) β€” treating as sequential barrier", - tool_name, - type(function_args).__name__, - ) + logging.debug("Non-dict args for %s (%s) β€” treating as sequential barrier", tool_name, type(function_args).__name__) return None name, args = _peel_bridge_call(tool_name, function_args) @@ -147,17 +133,13 @@ def _batch_admission(tool_call, execution_cwd: Optional[Path]) -> tuple[str, Lis def _plan_tool_batch_segments(tool_calls, *, execution_cwd: Optional[Path] = None) -> List[tuple]: """Split a tool-call batch into ordered ``("parallel"|"sequential", calls)`` segments. - Segments preserve the model's call order exactly β€” a later call never - crosses an earlier barrier β€” so result ordering and side-effect boundaries - match fully-sequential execution. Barriers: ``_NEVER_PARALLEL_TOOLS``, - unparseable/non-dict args, and anything not parallel-safe (built-in list, - bridge lookups, opted-in MCP tools). Path-scoped tools join a run only when - their paths don't conflict with the run's reservations: reader↔reader - overlap commutes and stays parallel; any overlap involving a writer closes - the run so the call starts a NEW run after the conflicting one lands. - ``search_files`` reserves its root (default ``.``) as a reader. Parallel - runs shorter than two calls demote to sequential (the sequential executor - owns the richer inline dispatch); adjacent sequential segments merge. + Call order is preserved exactly (a later call never crosses an earlier barrier), so + result order and side-effect boundaries match fully-sequential execution. Barriers: + ``_NEVER_PARALLEL_TOOLS``, unparseable/non-dict args, anything not parallel-safe. + Path-scoped tools join a run only if they don't conflict with its reservations: + reader↔reader overlap stays parallel; any overlap involving a writer closes the run so + the call starts a NEW run after the conflicting one lands. Runs shorter than two calls + demote to sequential (it owns the richer inline dispatch); adjacent sequential merge. """ segments: List[tuple] = [] current: list = [] @@ -207,9 +189,8 @@ def _should_parallelize_tool_batch(tool_calls) -> bool: def _canonical_path(raw_path: str, execution_cwd: Optional[Path] = None) -> Path: - """Canonical, OS-aware path for overlap detection (realpath for symlinks on - existing components, normcase for case-insensitive platforms); relative - paths resolve against *execution_cwd* or ``Path.cwd()``.""" + """Canonical, OS-aware path for overlap detection (realpath + normcase); relative paths + resolve against *execution_cwd* or ``Path.cwd()``.""" expanded = Path(raw_path).expanduser() base = execution_cwd if execution_cwd is not None else Path.cwd() candidate = expanded if expanded.is_absolute() else base / expanded @@ -221,13 +202,9 @@ def _extract_parallel_scope_paths( function_args: dict, execution_cwd: Optional[Path] = None, ) -> List[Path]: - """Every canonical path this call reserves for overlap checks. - - *execution_cwd* should be the cwd the tool will actually use (may differ - from the process cwd on WSL / sandboxed backends). For V4A ``patch`` the - scope comes from patch-body file headers. An empty result means the scope - is unknown and the planner must treat the call as a sequential barrier. - """ + """Every canonical path this call reserves for overlap checks. *execution_cwd* is the cwd + the tool will actually use (may differ from the process cwd on WSL / sandboxed backends); + V4A ``patch`` scope comes from patch-body headers. Empty = unknown scope = barrier.""" if tool_name not in _PATH_SCOPED_TOOLS: return [] @@ -239,9 +216,7 @@ def _extract_parallel_scope_paths( if isinstance(raw_path, str) and raw_path.strip(): raw_paths.append(raw_path) elif tool_name == "search_files": - # search_files defaults its root to the cwd; reserve that rather than - # demoting every bare search to a barrier. - raw_paths.append(".") + raw_paths.append(".") # its default root; reserving it beats demoting every bare search # dict.fromkeys dedupes while preserving first-seen order. return list(dict.fromkeys( @@ -261,23 +236,18 @@ def _extract_parallel_scope_path( def _paths_overlap(left: Path, right: Path) -> bool: - """True when two already-canonical paths may refer to the same subtree.""" - left_parts = left.parts - right_parts = right.parts + """True when two already-canonical paths may refer to the same subtree (an empty path + overlaps nothing).""" + left_parts, right_parts = left.parts, right.parts if not left_parts or not right_parts: - # Empty paths are guarded upstream; only two non-empty equal prefixes overlap. - return bool(left_parts) == bool(right_parts) and bool(left_parts) + return False common_len = min(len(left_parts), len(right_parts)) return left_parts[:common_len] == right_parts[:common_len] def _is_multimodal_tool_result(value: Any) -> bool: """True for the multimodal envelope: dict with ``_multimodal=True`` and a ``content`` list.""" - return ( - isinstance(value, dict) - and value.get("_multimodal") is True - and isinstance(value.get("content"), list) - ) + return isinstance(value, dict) and value.get("_multimodal") is True and isinstance(value.get("content"), list) def _is_text_part(p: Any) -> bool: @@ -322,12 +292,8 @@ _V4A_MOVE_HEADER = re.compile(r'^\*\*\*\s*Move\s+File:\s*(.+?)\s*->\s*(.+)$', re def _extract_file_mutation_targets(tool_name: str, args: Dict[str, Any]) -> List[str]: - """File paths a ``write_file`` / ``patch`` call targets. - - Replace mode uses ``args["path"]``; V4A patch mode parses the - ``*** Update/Add/Delete/Move File:`` headers so each file in a multi-file - patch is tracked separately. - """ + """File paths a ``write_file`` / ``patch`` call targets: ``args["path"]`` in replace mode, + every ``*** Update/Add/Delete/Move File:`` header in V4A patch mode.""" if tool_name not in _FILE_MUTATING_TOOLS: return [] mode = "replace" if tool_name == "write_file" else (args.get("mode") or "replace") @@ -389,8 +355,8 @@ def _extract_error_preview(result: Any, max_len: int = 180) -> str: def _trajectory_normalize_msg(msg: Dict[str, Any]) -> Dict[str, Any]: - """Shallow copy with image blobs stripped for trajectory saving: multimodal - results become their text summary, image parts become ``[screenshot]``.""" + """Shallow copy for trajectory saving: multimodal results become their text summary, + image parts become ``[screenshot]``.""" if not isinstance(msg, dict): return msg content = msg.get("content") @@ -420,19 +386,14 @@ def make_tool_result_message( *, effect_disposition: str | None = None, ) -> dict: - """Build a tool-result message with the OpenAI ``name`` field (wire format) - and the internal ``tool_name`` field (session DB). - - Content from high-risk tools (web_extract, web_search, browser_*, mcp_*) is - wrapped in untrusted-data delimiters β€” the architectural defense against - indirect prompt injection; see ``_maybe_wrap_untrusted``. + """Build a tool-result message: OpenAI ``name`` (wire format) plus internal ``tool_name`` + (session DB). High-risk tool content (web_extract, web_search, browser_*, mcp_*) is + wrapped in untrusted-data delimiters β€” the defense against indirect prompt injection. """ # Replay-recovery callers bypass the executor's canonical-id helper, so normalize here too. tool_call_id = _normalize_tool_call_id(tool_call_id) - - # Order matters: detect elision on the RAW content and append the notice - # first, THEN wrap, so the notice sits inside the untrusted block next to - # the data it describes β€” once, at construction time (cache-safe). + # Elision notice is appended to the RAW content first, THEN wrapped, so it sits inside + # the untrusted block next to the data it describes β€” once, at construction (cache-safe). wrapped = _maybe_wrap_untrusted(name, _maybe_append_elision_notice(name, content)) message = stamp_message_timestamp({ "role": "tool", @@ -453,14 +414,12 @@ def make_tool_result_message( return message -# Tools whose results carry attacker-controllable content. Short outputs -# (under 32 chars) skip wrapping: the overhead outweighs any injection risk. +# Tools whose results carry attacker-controllable content; outputs under 32 chars skip wrapping. _UNTRUSTED_TOOL_NAMES = frozenset({"web_extract", "web_search"}) _UNTRUSTED_TOOL_PREFIXES = ("browser_", "mcp_") _UNTRUSTED_WRAP_MIN_CHARS = 32 -# Case-insensitive so attacker content can't forge or prematurely close the -# boundary with a differently-cased tag the model would still read as one. +# Case-insensitive so a differently-cased tag can't forge or prematurely close the boundary. _DELIMITER_TOKEN_RE = re.compile(r"untrusted_tool_result", re.IGNORECASE) @@ -472,21 +431,16 @@ def _is_text_item(item: Any) -> bool: return _is_text_part(item) and isinstance(item.get("text"), str) -# --- Upstream-elision detection --- -# Some MCP servers elide data SERVER-SIDE and mark it inside the payload -# ('...13 more items', '"has_more": true', 'saved to sandbox', 'data_preview' -# envelopes). The result looks structurally complete, so models treat the -# visible slice as the whole dataset. Conservative, explicit markers only β€” -# not a generic truncation heuristic. One notice is appended at construction -# time, never mutated later (prompt-cache safe). +# Some MCP servers elide data SERVER-SIDE and mark it inside a structurally complete payload, +# so models treat the visible slice as the whole dataset. Conservative explicit markers only β€” +# not a generic truncation heuristic; the notice is appended once at construction (cache-safe). _UPSTREAM_ELISION_PATTERNS = ( re.compile(r"\.\.\.\s*\d+\s+more\s+items?", re.IGNORECASE), re.compile(r'"has_more"\s*:\s*true', re.IGNORECASE), re.compile(r"saved to sandbox", re.IGNORECASE), re.compile(r"data_preview", re.IGNORECASE), ) -# Tiny results can't hide an elided enumeration; markers for the sizes that -# matter (20-50K) are always inside the first 64KB. +# Tiny results can't hide an elided enumeration; markers for the sizes that matter sit in the first 64KB. _ELISION_SCAN_MIN_CHARS = 1_000 _ELISION_SCAN_MAX_CHARS = 65_536 @@ -513,49 +467,39 @@ def _maybe_append_elision_notice(name: str, content: Any) -> Any: def _tool_output_risk_metadata(name: str, content: Any) -> Optional[Dict[str, Any]]: - """Internal-only advisory classification of attacker-controlled output. - - Records deterministic finding ids, never blocks or redacts, and omits the scanned text. - """ + """Internal-only advisory classification of attacker-controlled output: deterministic + finding ids, never blocks or redacts, omits the scanned text.""" if not _is_untrusted_tool(name): return None if isinstance(content, str): text_parts = [content] elif isinstance(content, list): text_parts = [item["text"] for item in content if _is_text_item(item)] - if not text_parts: - return None else: return None + if not text_parts: + return None findings: List[str] = [] for text in text_parts: for finding in scan_for_threats(text, scope="context"): if finding not in findings: findings.append(finding) - return { - "risk": "high" if findings else "low", - "findings": findings, - "redacted": False, - } + return {"risk": "high" if findings else "low", "findings": findings, "redacted": False} def _neutralize_delimiters(content: str) -> str: - """Defang embedded ``untrusted_tool_result`` tokens so poisoned content - can't close the trust boundary early (hyphens keep it readable but non-matching).""" + """Defang embedded ``untrusted_tool_result`` tokens so poisoned content can't close the + trust boundary early (hyphens keep it readable but non-matching).""" return _DELIMITER_TOKEN_RE.sub("untrusted-tool-result", content) def _maybe_wrap_untrusted(name: str, content: Any) -> Any: - """Wrap high-risk tool content in untrusted-data delimiters. - - Strings are neutralized and wrapped in exactly one block; text parts of a - multimodal list are wrapped individually (non-text parts preserved, outer - list rebuilt β€” compare by value, not ``is``). Unchanged when the tool is - not high-risk, the content is neither str nor list, or a string is too - short. There is deliberately no "already wrapped" fast-path: it would be - attacker-forgeable, so harmless re-wrapping is the safe choice. - """ + """Wrap high-risk tool content in untrusted-data delimiters: strings are neutralized and + wrapped in exactly one block; text parts of a multimodal list are wrapped individually + (outer list rebuilt β€” compare by value, not ``is``). Unchanged for non-high-risk tools, + non-str/list content, or short strings. Deliberately no "already wrapped" fast-path: + it would be attacker-forgeable, so harmless re-wrapping is the safe choice.""" if not _is_untrusted_tool(name): return content if isinstance(content, str): From 3ca295513838e8be885049d841e01a26d979072e Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 2 Sep 2026 19:17:45 -0700 Subject: [PATCH 07/10] refactor(agent/tool_executor): concurrent workers index parsed_calls directly; compact gate/auth helpers --- agent/tool_executor.py | 92 +++++++++++++++--------------------------- 1 file changed, 33 insertions(+), 59 deletions(-) diff --git a/agent/tool_executor.py b/agent/tool_executor.py index 81dc766379..4f23297298 100644 --- a/agent/tool_executor.py +++ b/agent/tool_executor.py @@ -140,12 +140,7 @@ def _parse_tool_arguments(raw_arguments: Any) -> tuple[dict, Optional[str]]: if isinstance(arguments, dict): return arguments, None return {}, json.dumps( - { - "error": "Invalid tool arguments", - "message": ( - "Tool arguments must be a valid JSON object; tool was not executed." - ), - }, + {"error": "Invalid tool arguments", "message": "Tool arguments must be a valid JSON object; tool was not executed."}, ensure_ascii=False, ) @@ -417,10 +412,10 @@ class _ParsedCall: def _parse_tool_call(agent, tool_call, *, flatten_probe: bool = False) -> _ParsedCall: name = _canonical_tool_name(tool_call.function.name) args, parse_error = _parse_tool_arguments(tool_call.function.arguments) - if parse_error is not None: - return _ParsedCall(tool_call, name, args, [], parse_error, None) - name, args, scope_block = _unwrap_tool_search_call(agent, name, args, flatten_probe=flatten_probe) - return _ParsedCall(tool_call, name, args, [], None, scope_block) + scope_block = None + if parse_error is None: + name, args, scope_block = _unwrap_tool_search_call(agent, name, args, flatten_probe=flatten_probe) + return _ParsedCall(tool_call, name, args, [], parse_error, scope_block) @dataclass @@ -455,11 +450,11 @@ class _ConcurrentToolAuthorizationGate: self._lock_timeout = _authorization_gate_lock_timeout() if lock_timeout is None else lock_timeout self._session_key = session_key if self._session_key is None: + # Snapshot on the SUBMITTING thread: excluded_seconds() is polled from the + # batch wait loop, whose context may differ from the workers'. try: from tools.approval import get_current_session_key - # Snapshot on the SUBMITTING thread: excluded_seconds() is polled from the - # batch wait loop, whose context may differ from the workers'. self._session_key = get_current_session_key() except Exception: logger.debug( @@ -478,8 +473,7 @@ class _ConcurrentToolAuthorizationGate: return 0.0 def run(self, callback): - acquired = self._serialization_lock.acquire(timeout=self._lock_timeout) - if not acquired: + if not self._serialization_lock.acquire(timeout=self._lock_timeout): logger.warning( "authorization gate lock not acquired after %.1fs " "(holder wedged in a pre_tool_call plugin or approval " @@ -1127,17 +1121,13 @@ class _WorkerStartOnce: ``_BatchAbandoned`` (instead of dispatching late) when the batch was abandoned.""" def __init__(self, gate: _StartOrderGate, order: int, tool_name: str) -> None: - self._gate, self._order, self._tool_name = gate, order, tool_name - self._advanced = False + self._gate, self._order, self._tool_name, self._advanced = gate, order, tool_name, False def advance(self, callback=None) -> None: if self._advanced: return - try: - proceed = self._gate.begin_in_order(self._order, callback, tool_name=self._tool_name) - finally: - self._advanced = True - if not proceed: + self._advanced = True + if not self._gate.begin_in_order(self._order, callback, tool_name=self._tool_name): raise _BatchAbandoned(self._tool_name) @@ -1210,54 +1200,44 @@ class _ConcurrentBatch: logger.info("tool %s completed (%.2fs, %d chars)", ref.name, duration, len(result)) return _ToolOutcome(ref.name, ref.args, result, duration, is_error, blocked, ref.trace) - def run_worker(self, index, tool_call, function_name, function_args, middleware_trace, scope_block, start_order): + def run_worker(self, index: int, start_order: int) -> None: """Worker function executed in a thread.""" - agent = self.agent + agent, pc = self.agent, self.parsed_calls[index] with _registered_tool_worker(agent) as _worker_tid: # An interrupt may have fanned out before our registration; apply it to our tid. if agent._interrupt_requested: _interrupt_worker_tids(agent, [_worker_tid], reason=getattr(agent, "_tool_interrupt_reason", None)) _set_worker_activity_callback(agent) - start_gate = _WorkerStartOnce(self.gate, start_order, function_name) - ref = _ToolCallRef(function_name, function_args, self.effective_task_id, _pairing_tool_call_id(tool_call), middleware_trace) + start_gate = _WorkerStartOnce(self.gate, start_order, pc.name) try: - outcome = self._dispatch_worker(index, ref, scope_block, start_gate) + outcome = self._dispatch_worker(index, pc.ref(self.effective_task_id), pc.scope_block, start_gate) if outcome is not None: self.results[index] = outcome finally: with contextlib.suppress(_BatchAbandoned): start_gate.advance() # keep later-ordered workers moving - def submit_all(self, executor, runnable_calls) -> tuple[list, dict]: - """Submit every runnable call; on interpreter shutdown, synthesize error results + def submit_all(self, executor, runnable: list[int]) -> tuple[list, dict]: + """Submit every runnable slot; on interpreter shutdown, synthesize error results for the unsubmitted remainder instead of raising. ``propagate_context_to_thread`` carries turn ContextVars and thread-local approval/sudo callbacks into the worker.""" futures = [] future_to_index = {} - for submit_index, (i, tc, name, args, scope_block) in enumerate(runnable_calls): + for submit_index, i in enumerate(runnable): try: - f = executor.submit( - propagate_context_to_thread(self.run_worker), - i, tc, name, args, self.parsed_calls[i].middleware_trace, scope_block, submit_index, - ) + f = executor.submit(propagate_context_to_thread(self.run_worker), i, submit_index) except RuntimeError as submit_error: if not _is_interpreter_shutdown_submit_error(submit_error): raise - skipped_calls = runnable_calls[submit_index:] + skipped = runnable[submit_index:] logger.warning( - "interpreter shutdown while scheduling concurrent tools; skipping %d unsubmitted tool(s)", - len(skipped_calls), + "interpreter shutdown while scheduling concurrent tools; skipping %d unsubmitted tool(s)", len(skipped), ) - for skipped_i, _tc, skipped_name, skipped_args, _scope_block in skipped_calls: + for skipped_i in skipped: + pc = self.parsed_calls[skipped_i] if self.results[skipped_i] is None: - result = ( - f"Error executing tool '{skipped_name}': " - "Python interpreter is shutting down; tool was not started" - ) - self.results[skipped_i] = _ToolOutcome( - skipped_name, skipped_args, result, 0.0, True, False, - self.parsed_calls[skipped_i].middleware_trace, - ) + result = f"Error executing tool '{pc.name}': Python interpreter is shutting down; tool was not started" + self.results[skipped_i] = _ToolOutcome(pc.name, pc.args, result, 0.0, True, False, pc.middleware_trace) break futures.append(f) future_to_index[f] = i @@ -1274,15 +1254,13 @@ class _ConcurrentBatch: while True: wait_timeout = 5.0 if deadline is not None: - effective_deadline = deadline + self.authorization_gate.excluded_seconds() - remaining = effective_deadline - time.monotonic() + remaining = deadline + self.authorization_gate.excluded_seconds() - time.monotonic() if remaining <= 0: - done, not_done = set(), {f for f in futures if not f.done()} + not_done = {f for f in futures if not f.done()} else: wait_timeout = min(wait_timeout, remaining) - done, not_done = concurrent.futures.wait(futures, timeout=wait_timeout) - else: - done, not_done = concurrent.futures.wait(futures, timeout=wait_timeout) + if deadline is None or remaining > 0: + _done, not_done = concurrent.futures.wait(futures, timeout=wait_timeout) if not not_done: return False @@ -1328,21 +1306,17 @@ class _ConcurrentBatch: def run(self) -> None: """Dispatch the runnable calls on a daemon pool and wait for the batch.""" - runnable_calls = [ - (i, pc.tool_call, pc.name, pc.args, pc.scope_block) - for i, pc in enumerate(self.parsed_calls) - if pc.parse_error is None - ] - if not runnable_calls: + runnable = [i for i, pc in enumerate(self.parsed_calls) if pc.parse_error is None] + if not runnable: return deadline = time.monotonic() + self.timeout_s if self.timeout_s is not None else None - max_workers = _max_workers_for_tool_batch(runnable_calls) + max_workers = _max_workers_for_tool_batch([(i, None, self.parsed_calls[i].name) for i in runnable]) # Daemon workers: the stdlib pool's atexit join would let one wedged tool block exit. from tools.daemon_pool import DaemonThreadPoolExecutor executor = DaemonThreadPoolExecutor(max_workers=max_workers) abandon_executor = False try: - futures, future_to_index = self.submit_all(executor, runnable_calls) + futures, future_to_index = self.submit_all(executor, runnable) abandon_executor = self.await_completion(futures, future_to_index, deadline) finally: # Every abandoning exit releases gate-parked workers and leaves wedged threads From 8efd6a682a590a105832705e27b7aa818a2824a8 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 2 Sep 2026 19:22:02 -0700 Subject: [PATCH 08/10] refactor(agent/tool_executor): fold observe into _commit_tool_result; _ToolOutcome carries its ref; merge sequential abandon branches --- agent/tool_executor.py | 182 +++++++++++++++-------------------------- 1 file changed, 68 insertions(+), 114 deletions(-) diff --git a/agent/tool_executor.py b/agent/tool_executor.py index 4f23297298..9b94eea950 100644 --- a/agent/tool_executor.py +++ b/agent/tool_executor.py @@ -825,33 +825,28 @@ def _run_sequential_tool_execution_middleware( concurrent.futures.wait([future], timeout=3.0) if future.done() and not future.cancelled(): return future.result() - abandoned = True - future.cancel() interrupt_reason = getattr(agent, "_tool_interrupt_reason", None) or "interrupt requested" message = f"[Tool execution cancelled β€” {function_name} was abandoned: {interrupt_reason}]" logger.info( "sequential tool %s abandoned due to %s (%.1fs elapsed)", function_name, interrupt_reason, time.monotonic() - started, ) - return _abandoned_sequential_result( - agent, ref, message, _ToolCancelledResult, - duration_ms=int((time.monotonic() - started) * 1000), - status="cancelled", - error_type="tool_interrupted", - error_message=f"Tool execution cancelled: {interrupt_reason}", + result_cls, outcome = _ToolCancelledResult, dict( + duration_ms=int((time.monotonic() - started) * 1000), status="cancelled", + error_type="tool_interrupted", error_message=f"Tool execution cancelled: {interrupt_reason}", + ) + else: + assert timeout_s is not None # only reachable when a deadline exists + message = f"Error executing tool '{function_name}': timed out after {timeout_s:.1f}s" + logger.warning("sequential tool %s timed out after %.1fs", function_name, timeout_s) + result_cls, outcome = _ToolTimeoutResult, dict( + duration_ms=int(timeout_s * 1000), status="timeout", error_type="tool_timeout", error_message=message, ) - - # Only reachable when a deadline exists (interrupted returns above). - assert timeout_s is not None abandoned = True - message = f"Error executing tool '{function_name}': timed out after {timeout_s:.1f}s" - logger.warning("sequential tool %s timed out after %.1fs", function_name, timeout_s) future.cancel() - _interrupt_worker_tids(agent, worker_tid) - return _abandoned_sequential_result( - agent, ref, message, _ToolTimeoutResult, - duration_ms=int(timeout_s * 1000), status="timeout", error_type="tool_timeout", error_message=message, - ) + if state == "timeout": + _interrupt_worker_tids(agent, worker_tid) + return _abandoned_sequential_result(agent, ref, message, result_cls, **outcome) finally: # Never join a wedged worker (daemon pool also keeps it out of the atexit join). executor.shutdown(wait=not abandoned, cancel_futures=abandoned) @@ -935,12 +930,39 @@ def _commit_tool_result( is_error: bool, blocked: bool, effect_disposition, + observed: bool = False, + error_preview: Callable[[Any], Any] = lambda result: result, + success_log_chars: Optional[int] = None, + verbose_text: Callable[[Any], Any] = lambda result: result, ): - """Mark the tool done; persist/spill, hint, wrap and append its result; flush the session - DB; then project ``tool.completed``. Returns ``(persisted_result, display_result, - risk_metadata)`` (``display_result`` = pre-persist content for UI previews) or ``None`` - when the flush failed (the caller must stop the batch).""" + """Observe (``observed`` results only) and log the outcome; mark the tool done; persist/ + spill, hint, wrap and append the result; flush the session DB; project ``tool.completed``. + + Blocked calls never ran, so they are neither guardrail-observed nor fed to the file- + mutation verifier; ``success_log_chars`` (sequential path) also logs the completion line. + Returns ``(persisted_result, display_result, risk_metadata)`` (``display_result`` = + pre-persist content for UI previews) or ``None`` when the flush failed (stop the batch). + """ function_name, function_args, tool_call_id, effective_task_id = ref.name, ref.args, ref.call_id, ref.task_id + if observed: + if not blocked: + function_result = agent._append_guardrail_observation( + function_name, function_args, function_result, failed=is_error, tool_call_id=tool_call_id, + ) + if is_error: + logger.warning("Tool %s returned error (%.2fs): %s", function_name, tool_duration, error_preview(function_result)) + elif success_log_chars is not None: + logger.info("tool %s completed (%.2fs, %d chars)", function_name, tool_duration, success_log_chars) + if not blocked: + try: + agent._record_file_mutation_result(function_name, function_args, function_result, is_error) + except Exception as _ver_err: + logging.debug("file-mutation verifier record failed: %s", _ver_err) + if agent.verbose_logging: + logging.debug("Tool %s completed in %.2fs", function_name, tool_duration) + _log_result = verbose_text(function_result) + logging.debug("Tool result (%d chars): %s", len(_log_result), _log_result) + agent._current_tool = None _status_suffix = " (error)" if is_error else "" agent._touch_activity(f"tool completed: {function_name} ({tool_duration:.1f}s){_status_suffix}") @@ -982,52 +1004,6 @@ def _commit_tool_result( return persisted_result, function_result, tool_message.get("_tool_output_risk") -def _observe_and_commit_tool_result( - agent, - messages: list, - ref: _ToolCallRef, - function_result, - *, - budget: BudgetConfig, - tool_duration: float, - is_error: bool, - blocked: bool, - effect_disposition, - error_preview: Callable[[Any], Any], - success_log_chars: Optional[int] = None, - verbose_text: Callable[[Any], Any] = lambda result: result, -): - """Guardrail-observe a result that actually ran, log its outcome, feed the turn-end - file-mutation verifier, then ``_commit_tool_result`` it. Blocked calls never ran, so - they count as neither failure nor success and are not observed; ``success_log_chars`` - (sequential path) also logs the completion line.""" - function_name, function_args, tool_call_id = ref.name, ref.args, ref.call_id - if not blocked: - function_result = agent._append_guardrail_observation( - function_name, function_args, function_result, failed=is_error, tool_call_id=tool_call_id, - ) - if is_error: - logger.warning("Tool %s returned error (%.2fs): %s", function_name, tool_duration, error_preview(function_result)) - elif success_log_chars is not None: - logger.info("tool %s completed (%.2fs, %d chars)", function_name, tool_duration, success_log_chars) - if not blocked: - try: - agent._record_file_mutation_result(function_name, function_args, function_result, is_error) - except Exception as _ver_err: - logging.debug("file-mutation verifier record failed: %s", _ver_err) - - if agent.verbose_logging: - logging.debug("Tool %s completed in %.2fs", function_name, tool_duration) - _log_result = verbose_text(function_result) - logging.debug("Tool result (%d chars): %s", len(_log_result), _log_result) - - return _commit_tool_result( - agent, messages, ref, function_result, - budget=budget, tool_duration=tool_duration, is_error=is_error, blocked=blocked, - effect_disposition=effect_disposition, - ) - - def _finalize_tool_batch(agent, messages: list, effective_task_id: str, num_tools: int, budget: BudgetConfig) -> None: """Per-turn aggregate budget enforcement, then /steer injection β€” in that order, so the steer marker is never truncated/discarded when enforcement replaces a result.""" @@ -1057,15 +1033,13 @@ def _print_tool_completed(agent, index: int, tool_duration: float, result) -> No @dataclass class _ToolOutcome: - """One finished worker slot of a concurrent batch.""" + """One finished worker slot of a concurrent batch (``ref`` holds the final name/args/trace).""" - name: str - args: dict + ref: _ToolCallRef result: Any duration: float is_error: bool blocked: bool - middleware_trace: list def _start_order_gate_timeout(batch_timeout: float | None) -> float: @@ -1144,7 +1118,7 @@ class _ConcurrentBatch: self.results: list[Optional[_ToolOutcome]] = [None] * len(parsed_calls) for i, pc in enumerate(parsed_calls): if pc.parse_error is not None: - self.results[i] = _ToolOutcome(pc.name, pc.args, pc.parse_error, 0.0, True, True, pc.middleware_trace) + self.results[i] = _ToolOutcome(pc.ref(effective_task_id), pc.parse_error, 0.0, True, True) self.gate = _StartOrderGate(_start_order_gate_timeout(timeout_s)) self.authorization_gate = _ConcurrentToolAuthorizationGate() self.timed_out_indices: set[int] = set() @@ -1186,7 +1160,7 @@ class _ConcurrentBatch: result = ref.emit_cancelled(agent, start) duration = time.time() - start logger.info("tool %s cancelled (%.2fs)", ref.name, duration) - return _ToolOutcome(ref.name, ref.args, result, duration, True, False, ref.trace) + return _ToolOutcome(ref, result, duration, True, False) except Exception as tool_error: result = f"Error executing tool '{ref.name}': {tool_error}" logger.error("_invoke_tool raised for %s: %s", ref.name, tool_error, exc_info=True) @@ -1198,7 +1172,7 @@ class _ConcurrentBatch: logger.info("tool %s failed (%.2fs): %s", ref.name, duration, result[:200]) else: logger.info("tool %s completed (%.2fs, %d chars)", ref.name, duration, len(result)) - return _ToolOutcome(ref.name, ref.args, result, duration, is_error, blocked, ref.trace) + return _ToolOutcome(ref, result, duration, is_error, blocked) def run_worker(self, index: int, start_order: int) -> None: """Worker function executed in a thread.""" @@ -1234,10 +1208,10 @@ class _ConcurrentBatch: "interpreter shutdown while scheduling concurrent tools; skipping %d unsubmitted tool(s)", len(skipped), ) for skipped_i in skipped: - pc = self.parsed_calls[skipped_i] + ref = self.parsed_calls[skipped_i].ref(self.effective_task_id) if self.results[skipped_i] is None: - result = f"Error executing tool '{pc.name}': Python interpreter is shutting down; tool was not started" - self.results[skipped_i] = _ToolOutcome(pc.name, pc.args, result, 0.0, True, False, pc.middleware_trace) + result = f"Error executing tool '{ref.name}': Python interpreter is shutting down; tool was not started" + self.results[skipped_i] = _ToolOutcome(ref, result, 0.0, True, False) break futures.append(f) future_to_index[f] = i @@ -1352,29 +1326,24 @@ def _append_batch_results(agent, messages: list, effective_task_id: str, batch: failed flush (the caller must stop the batch).""" for i, pc in enumerate(batch.parsed_calls): r = batch.results[i] - ref = pc.ref(effective_task_id) # A worker may finish between the deadline snapshot and this loop; # prefer its real result over a fabricated timeout. if r is None: - blocked = False + ref, is_error, blocked = pc.ref(effective_task_id), True, False function_result, tool_duration, effect_disposition = _unfinished_tool_result( agent, ref, timed_out=i in batch.timed_out_indices, timeout_s=batch.timeout_s, ) - committed = _commit_tool_result( - agent, messages, ref, function_result, - budget=budget, tool_duration=tool_duration, is_error=True, blocked=False, - effect_disposition=effect_disposition, - ) else: - ref.name, ref.args, ref.trace, tool_duration, blocked = r.name, r.args, r.middleware_trace, r.duration, r.blocked + ref, function_result, tool_duration, is_error, blocked = r.ref, r.result, r.duration, r.is_error, r.blocked + effect_disposition = "none" if blocked else None if pc.parse_error is not None: ref.emit_invalid_arguments(agent, r.result) - committed = _observe_and_commit_tool_result( - agent, messages, ref, r.result, - budget=budget, tool_duration=tool_duration, is_error=r.is_error, blocked=blocked, - effect_disposition="none" if blocked else None, - error_preview=lambda res: _multimodal_text_summary(res)[:200], - ) + committed = _commit_tool_result( + agent, messages, ref, function_result, + budget=budget, tool_duration=tool_duration, is_error=is_error, blocked=blocked, + effect_disposition=effect_disposition, observed=r is not None, + error_preview=lambda res: _multimodal_text_summary(res)[:200], + ) if committed is None: return False _persisted, display_function_result, risk_metadata = committed @@ -1492,19 +1461,13 @@ class _SequentialDispatch: finish_in_finally: bool = True -def _resolve_sequential_dispatch( - agent, - *, - function_name: str, - function_args: dict, - messages: list, - effective_task_id: str, - tool_call_id: str, - middleware_trace: list, -) -> _SequentialDispatch: +def _resolve_sequential_dispatch(agent, ref: _ToolCallRef, messages: list) -> _SequentialDispatch: """Pick the execute callable for one sequential call and start its spinner. Precedence: inline agent-level tools, delegate_task, context-engine tools, memory-provider tools, then the registry.""" + function_name, function_args, effective_task_id, tool_call_id, middleware_trace = ( + ref.name, ref.args, ref.task_id, ref.call_id, ref.trace, + ) if function_name != "delegate_task" and function_name in INLINE_TOOL_EXECUTORS: # Agent-level tools that need live AIAgent state; table shared with invoke_tool. inline_executor = INLINE_TOOL_EXECUTORS[function_name] @@ -1654,10 +1617,10 @@ def _publish_sequential_result(agent, messages: list, ref: _ToolCallRef, managed # observer is suppressed); also stops an abandoned timeout worker reporting late. if not managed.blocked and not _execution_timed_out: ref.emit_post(agent, function_result, duration_ms=int(tool_duration * 1000)) - committed = _observe_and_commit_tool_result( + committed = _commit_tool_result( agent, messages, ref, function_result, budget=budget, tool_duration=tool_duration, is_error=_is_error_result, blocked=managed.blocked, - effect_disposition="unknown" if _execution_timed_out else None, + effect_disposition="unknown" if _execution_timed_out else None, observed=True, error_preview=lambda res: res[:200] if isinstance(res, str) and not agent.verbose_logging else res, success_log_chars=_result_len, verbose_text=_multimodal_text_summary, @@ -1680,7 +1643,6 @@ def execute_tool_calls_sequential(agent, assistant_message, messages: list, effe tool_calls = assistant_message.tool_calls for i, tool_call in enumerate(tool_calls, 1): - tool_call_id = _pairing_tool_call_id(tool_call) if getattr(agent, "_incremental_persistence_failed", False): return # Check interrupt BEFORE each tool so a "stop" during the previous one skips the rest. @@ -1704,15 +1666,7 @@ def execute_tool_calls_sequential(agent, assistant_message, messages: list, effe continue tool_start_time = time.time() - dispatch = _resolve_sequential_dispatch( - agent, - function_name=ref.name, - function_args=ref.args, - messages=messages, - effective_task_id=effective_task_id, - tool_call_id=tool_call_id, - middleware_trace=ref.trace, - ) + dispatch = _resolve_sequential_dispatch(agent, ref, messages) managed, tool_duration = _run_sequential_call( agent, dispatch, ref, scope_block=pc.scope_block, From a64889ef3d9e31e8c2812c7c0a4b7ec8a4f42fcd Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 2 Sep 2026 19:25:43 -0700 Subject: [PATCH 09/10] refactor(agent/tool_executor,tool_dispatch_helpers): compact spinner/dispatch helpers, planner closures, envelope helpers --- agent/tool_dispatch_helpers.py | 73 ++++++++++------------------------ agent/tool_executor.py | 53 +++++++++--------------- 2 files changed, 40 insertions(+), 86 deletions(-) diff --git a/agent/tool_dispatch_helpers.py b/agent/tool_dispatch_helpers.py index fd56cb0477..42209f7447 100644 --- a/agent/tool_dispatch_helpers.py +++ b/agent/tool_dispatch_helpers.py @@ -145,20 +145,19 @@ def _plan_tool_batch_segments(tool_calls, *, execution_cwd: Optional[Path] = Non current: list = [] reserved_paths: list[tuple[Path, bool]] = [] # (canonical_path, is_writer) for the current run + def _extend_sequential(calls: list) -> None: + if segments and segments[-1][0] == "sequential": + segments[-1][1].extend(calls) + else: + segments.append(("sequential", list(calls))) + def _close_parallel() -> None: nonlocal current, reserved_paths if len(current) >= 2: segments.append(("parallel", current)) elif current: _extend_sequential(current) - current = [] - reserved_paths = [] - - def _extend_sequential(calls: list) -> None: - if segments and segments[-1][0] == "sequential": - segments[-1][1].extend(calls) - else: - segments.append(("sequential", list(calls))) + current, reserved_paths = [], [] for tool_call in tool_calls: admission = _batch_admission(tool_call, execution_cwd) @@ -208,21 +207,14 @@ def _extract_parallel_scope_paths( if tool_name not in _PATH_SCOPED_TOOLS: return [] - raw_paths: List[str] = [] if tool_name == "patch" and (function_args.get("mode") or "replace") == "patch": - raw_paths.extend(_extract_file_mutation_targets(tool_name, function_args)) + raw_paths = _extract_file_mutation_targets(tool_name, function_args) else: raw_path = function_args.get("path") - if isinstance(raw_path, str) and raw_path.strip(): - raw_paths.append(raw_path) - elif tool_name == "search_files": - raw_paths.append(".") # its default root; reserving it beats demoting every bare search - + # search_files defaults its root to the cwd; reserving it beats demoting every bare search. + raw_paths = [raw_path] if isinstance(raw_path, str) and raw_path.strip() else ["."] if tool_name == "search_files" else [] # dict.fromkeys dedupes while preserving first-seen order. - return list(dict.fromkeys( - _canonical_path(raw, execution_cwd) - for raw in raw_paths if isinstance(raw, str) and raw.strip() - )) + return list(dict.fromkeys(_canonical_path(raw, execution_cwd) for raw in raw_paths if isinstance(raw, str) and raw.strip())) def _extract_parallel_scope_path( @@ -256,13 +248,13 @@ def _is_text_part(p: Any) -> bool: def _multimodal_text_summary(value: Any) -> str: """Plain-text view of a tool result (logging, previews, string-only providers).""" + if isinstance(value, str): + return value if _is_multimodal_tool_result(value): if value.get("text_summary"): return str(value["text_summary"]) parts = [str(p.get("text", "")) for p in value.get("content") or [] if _is_text_part(p)] return "\n".join(parts) if parts else "[multimodal tool result]" - if isinstance(value, str): - return value try: return json.dumps(value, default=str) except Exception: @@ -327,13 +319,10 @@ def _extract_landed_file_mutation_paths( return targets if not isinstance(data, dict): return targets - files = data.get("files_modified") landed = [str(p) for p in files if p] if isinstance(files, list) else [] - if landed: - return landed resolved = data.get("resolved_path") - return [str(resolved)] if resolved else targets + return landed or ([str(resolved)] if resolved else targets) def _extract_error_preview(result: Any, max_len: int = 180) -> str: @@ -364,9 +353,7 @@ def _trajectory_normalize_msg(msg: Dict[str, Any]) -> Dict[str, Any]: return {**msg, "content": _multimodal_text_summary(content)} if isinstance(content, list): return {**msg, "content": [ - {"type": "text", "text": "[screenshot]"} - if isinstance(p, dict) and p.get("type") in {"image", "image_url", "input_image"} - else p + {"type": "text", "text": "[screenshot]"} if isinstance(p, dict) and p.get("type") in {"image", "image_url", "input_image"} else p for p in content ]} return msg @@ -524,28 +511,12 @@ def _maybe_wrap_untrusted(name: str, content: Any) -> Any: __all__ = [ - "_NEVER_PARALLEL_TOOLS", - "_PARALLEL_SAFE_TOOLS", - "_PATH_SCOPED_TOOLS", - "_PATH_SCOPED_READERS", - "_PATH_SCOPED_WRITERS", - "_DESTRUCTIVE_PATTERNS", - "_REDIRECT_OVERWRITE", - "_is_destructive_command", - "_plan_tool_batch_segments", - "_should_parallelize_tool_batch", - "_canonical_path", - "_extract_parallel_scope_path", - "_extract_parallel_scope_paths", - "_paths_overlap", - "_is_multimodal_tool_result", - "_multimodal_text_summary", - "_append_subdir_hint_to_multimodal", - "_extract_file_mutation_targets", - "_extract_landed_file_mutation_paths", - "_extract_error_preview", - "_trajectory_normalize_msg", - "_detect_upstream_elision", - "_maybe_append_elision_notice", + "_NEVER_PARALLEL_TOOLS", "_PARALLEL_SAFE_TOOLS", "_PATH_SCOPED_TOOLS", "_PATH_SCOPED_READERS", + "_PATH_SCOPED_WRITERS", "_DESTRUCTIVE_PATTERNS", "_REDIRECT_OVERWRITE", "_is_destructive_command", + "_plan_tool_batch_segments", "_should_parallelize_tool_batch", "_canonical_path", + "_extract_parallel_scope_path", "_extract_parallel_scope_paths", "_paths_overlap", + "_is_multimodal_tool_result", "_multimodal_text_summary", "_append_subdir_hint_to_multimodal", + "_extract_file_mutation_targets", "_extract_landed_file_mutation_paths", "_extract_error_preview", + "_trajectory_normalize_msg", "_detect_upstream_elision", "_maybe_append_elision_notice", "make_tool_result_message", ] diff --git a/agent/tool_executor.py b/agent/tool_executor.py index 9b94eea950..89955bcd19 100644 --- a/agent/tool_executor.py +++ b/agent/tool_executor.py @@ -1395,8 +1395,7 @@ def execute_tool_calls_concurrent(agent, assistant_message, messages: list, effe finally: if spinner: finished = [r for r in batch.results if r is not None] - total_dur = sum(r.duration for r in finished) - spinner.stop(f"⚑ {len(finished)}/{num_tools} tools completed in {total_dur:.1f}s total") + spinner.stop(f"⚑ {len(finished)}/{num_tools} tools completed in {sum(r.duration for r in finished):.1f}s total") if not _append_batch_results(agent, messages, effective_task_id, batch, _tool_budget): return @@ -1410,15 +1409,12 @@ def execute_tool_calls_concurrent(agent, assistant_message, messages: list, effe def _start_quiet_tool_spinner(agent, function_name: str, function_args: dict, *, gate: bool = True, label: Optional[str] = None): """Start the quiet-mode kawaii spinner for one tool call, or return None; ``gate=False`` skips ``_should_start_quiet_spinner`` (context-engine tools always spin).""" - if not agent._should_emit_quiet_tool_messages(): - return None - if gate and not agent._should_start_quiet_spinner(): + if not agent._should_emit_quiet_tool_messages() or (gate and not agent._should_start_quiet_spinner()): return None face = random.choice(KawaiiSpinner.get_waiting_faces()) if label is None: - emoji = _get_tool_emoji(function_name) display_args = _redact_tool_args_for_display(function_name, function_args) or function_args - label = f"{emoji} {_build_tool_label(function_name, display_args) or function_name}" + label = f"{_get_tool_emoji(function_name)} {_build_tool_label(function_name, display_args) or function_name}" spinner = KawaiiSpinner(f"{face} {label}", spinner_type='dots', print_fn=agent._print_fn) spinner.start() return spinner @@ -1426,19 +1422,18 @@ def _start_quiet_tool_spinner(agent, function_name: str, function_args: dict, *, def _finish_quiet_tool_spinner(agent, spinner, function_name: str, function_args: dict, tool_duration: float, result) -> None: """Stop the spinner with the cute completion line, or print it when no spinner ran.""" - if spinner: - spinner.stop(_get_cute_tool_message_impl(function_name, function_args, tool_duration, result=result)) - elif agent._should_emit_quiet_tool_messages(): - agent._vprint(f" {_get_cute_tool_message_impl(function_name, function_args, tool_duration, result=result)}") + if spinner or agent._should_emit_quiet_tool_messages(): + cute = _get_cute_tool_message_impl(function_name, function_args, tool_duration, result=result) + spinner.stop(cute) if spinner else agent._vprint(f" {cute}") def _delegate_spinner_label(function_args: dict) -> str: - _action_arg = str(function_args.get("action") or "").strip().lower() - tasks_arg = function_args.get("tasks") - if _action_arg in ("list", "steer", "stop"): - return f"πŸ”€ subagent {_action_arg}" - if tasks_arg and isinstance(tasks_arg, list): - return f"πŸ”€ delegating {len(tasks_arg)} tasks Β· (/agents to monitor)" + action = str(function_args.get("action") or "").strip().lower() + tasks = function_args.get("tasks") + if action in ("list", "steer", "stop"): + return f"πŸ”€ subagent {action}" + if tasks and isinstance(tasks, list): + return f"πŸ”€ delegating {len(tasks)} tasks Β· (/agents to monitor)" goal_preview = (function_args.get("goal") or "")[:30] return f"πŸ”€ {goal_preview} Β· (/agents to monitor)" if goal_preview else "πŸ”€ delegating Β· (/agents to monitor)" @@ -1449,16 +1444,13 @@ class _SequentialDispatch: execute: Callable[[dict], Any] spinner: Any = None - # Passed through to the middleware runner; the registry closure reads this list. - middleware_trace_arg: Optional[list] = None - # None β†’ exceptions propagate (inline / delegate tools own their failures). - error_result: Optional[Callable[[Exception], str]] = None + middleware_trace_arg: Optional[list] = None # forwarded to the middleware runner (registry closure reads it) + error_result: Optional[Callable[[Exception], str]] = None # None β†’ exceptions propagate (inline/delegate own failures) error_log: str = "" handles_keyboard_interrupt: bool = False is_delegate: bool = False finish_spinner: bool = True - # Inline tools print their completion line only on success (no try/finally). - finish_in_finally: bool = True + finish_in_finally: bool = True # inline tools print their completion line only on success def _resolve_sequential_dispatch(agent, ref: _ToolCallRef, messages: list) -> _SequentialDispatch: @@ -1472,20 +1464,12 @@ def _resolve_sequential_dispatch(agent, ref: _ToolCallRef, messages: list) -> _S # Agent-level tools that need live AIAgent state; table shared with invoke_tool. inline_executor = INLINE_TOOL_EXECUTORS[function_name] inline_ctx = InlineToolContext(effective_task_id=effective_task_id, tool_call_id=tool_call_id, messages=messages) - return _SequentialDispatch( - execute=lambda next_args: inline_executor(agent, next_args, inline_ctx), - finish_in_finally=False, - ) + return _SequentialDispatch(lambda next_args: inline_executor(agent, next_args, inline_ctx), finish_in_finally=False) if function_name == "delegate_task": spinner = _start_quiet_tool_spinner(agent, function_name, function_args, label=_delegate_spinner_label(function_args)) agent._delegate_spinner = spinner - return _SequentialDispatch( - execute=lambda next_args: agent._dispatch_delegate_task(next_args), - spinner=spinner, - is_delegate=True, - ) + return _SequentialDispatch(agent._dispatch_delegate_task, spinner=spinner, is_delegate=True) if agent._context_engine_tool_names and function_name in agent._context_engine_tool_names: - # Context engine tools (lcm_grep, lcm_describe, lcm_expand, etc.) return _SequentialDispatch( execute=lambda next_args: agent.context_compressor.handle_tool_call(function_name, next_args, messages=messages), spinner=_start_quiet_tool_spinner(agent, function_name, function_args, gate=False), @@ -1493,8 +1477,7 @@ def _resolve_sequential_dispatch(agent, ref: _ToolCallRef, messages: list) -> _S error_log="context_engine.handle_tool_call raised for %s: %s", ) if agent._memory_manager and agent._memory_manager.has_tool(function_name): - # Memory provider tools (hindsight_retain, honcho_search, etc.) are not in the - # tool registry β€” route through MemoryManager. + # Memory-provider tools (hindsight_retain, honcho_search, ...) are not in the registry. return _SequentialDispatch( execute=lambda next_args: agent._memory_manager.handle_tool_call(function_name, next_args), spinner=_start_quiet_tool_spinner(agent, function_name, function_args), From 2252200b2abc8f8766fd45f72c2d5cf23204980d Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 2 Sep 2026 19:29:07 -0700 Subject: [PATCH 10/10] refactor(agent/tool_executor): contextlib.suppress for best-effort blocks; _preview helper; alias _pairing_tool_call_id --- agent/tool_executor.py | 76 ++++++++++++++---------------------------- 1 file changed, 25 insertions(+), 51 deletions(-) diff --git a/agent/tool_executor.py b/agent/tool_executor.py index 89955bcd19..a8f33ba90d 100644 --- a/agent/tool_executor.py +++ b/agent/tool_executor.py @@ -59,9 +59,7 @@ from tools.budget_config import BudgetConfig, DEFAULT_BUDGET, budget_for_context logger = logging.getLogger(__name__) -def _pairing_tool_call_id(tool_call: Any) -> str: - """Return the canonical id used by the persisted assistant message.""" - return coalesce_tool_call_id(tool_call) +_pairing_tool_call_id = coalesce_tool_call_id # canonical id used by the persisted assistant message def _tc_name(tool_call: Any) -> str: @@ -71,9 +69,7 @@ def _tc_name(tool_call: Any) -> str: def _record_persisted_path_for_stub(agent, tool_call_id: str, function_result) -> None: """Record the spillover file path so a later result-reference stub can't dangle (best-effort).""" try: - if not isinstance(function_result, str): - return - path = extract_persisted_path(function_result) + path = extract_persisted_path(function_result) if isinstance(function_result, str) else None if path: agent._tool_guardrails.record_persisted_result(tool_call_id, path) except Exception as exc: @@ -89,8 +85,9 @@ def _ensure_file_checkpoint(agent, function_name: str, function_args: dict, effe from tools.file_tools import _resolve_path_for_task resolved_path = _resolve_path_for_task(file_path, effective_task_id or "default") - work_dir = agent._checkpoint_mgr.get_working_dir_for_path(str(resolved_path)) - agent._checkpoint_mgr.ensure_checkpoint(work_dir, f"before {function_name}") + agent._checkpoint_mgr.ensure_checkpoint( + agent._checkpoint_mgr.get_working_dir_for_path(str(resolved_path)), f"before {function_name}", + ) def _budget_for_agent(agent) -> BudgetConfig: @@ -164,8 +161,7 @@ def _flush_session_db_after_tool_progress(agent, messages: list, *, stage: str) persisted = agent._flush_messages_to_session_db(messages) is not False if not persisted: agent._incremental_persistence_failed = True - # Flush recorded any classified cause at the catch site; only default - # to 'unknown' when nothing more specific exists. + # The flush recorded any classified cause; default to 'unknown' only if nothing more specific exists. if getattr(agent, "_last_persistence_error_cause", None) is None: agent._last_persistence_error_cause = "unknown" return persisted @@ -256,24 +252,18 @@ class _ToolCallRef: def emit_cancelled(self, agent, start_time: float) -> str: """Synthesize the ``cancelled`` result for a KeyboardInterrupt mid-tool and emit its hook.""" - result = json.dumps( - {"error": "Tool execution cancelled by user interrupt", "status": "cancelled"}, - ensure_ascii=False, - ) + message = "Tool execution cancelled by user interrupt" + result = json.dumps({"error": message, "status": "cancelled"}, ensure_ascii=False) self.emit_post( - agent, result, - duration_ms=int((time.time() - start_time) * 1000), - status="cancelled", - error_type="keyboard_interrupt", - error_message="Tool execution cancelled by user interrupt", + agent, result, duration_ms=int((time.time() - start_time) * 1000), + status="cancelled", error_type="keyboard_interrupt", error_message=message, ) return result def emit_invalid_arguments(self, agent, result: str) -> None: self.emit_post( agent, result, trace=[], - status="error", error_type="invalid_tool_arguments", - error_message="Tool arguments must be a valid JSON object", + status="error", error_type="invalid_tool_arguments", error_message="Tool arguments must be a valid JSON object", ) @@ -338,10 +328,8 @@ def _tool_search_scoped_names(agent) -> frozenset: ) or []) except Exception: names = frozenset() - try: + with contextlib.suppress(Exception): agent._tool_search_scope_cache = (cache_key, names) - except Exception: - pass return names @@ -503,10 +491,8 @@ def _registered_tool_worker(agent): finally: with agent._tool_worker_threads_lock: agent._tool_worker_threads.discard(tid) - try: + with contextlib.suppress(Exception): _ra()._set_interrupt(False, tid) - except Exception: - pass _NO_REASON = object() @@ -516,20 +502,16 @@ def _interrupt_worker_tids(agent, tids, *, reason=_NO_REASON) -> None: """Raise the interrupt bit on each worker tid (best-effort, via ``run_agent``).""" kwargs = {} if reason is _NO_REASON else {"reason": reason} for tid in tids: - try: + with contextlib.suppress(Exception): _ra()._set_interrupt(True, tid, **kwargs) - except Exception: - pass def _set_worker_activity_callback(agent) -> None: """The activity callback is thread-local: bind it on THIS thread so tool-layer heartbeats fire.""" - try: + with contextlib.suppress(Exception): from tools.environments.base import set_activity_callback set_activity_callback(agent._touch_activity) - except Exception: - pass # Must stay far below the gateway turn-inactivity timeout (default 1800s) so a silent tool never looks idle. @@ -574,9 +556,7 @@ def _blocked_tool_result(agent, ref: _ToolCallRef, *, block_message: Optional[st """Synthesize the result for a call blocked by scope/plugin (``block_message``) or by guardrail policy (``guardrail_decision``) and emit its terminal post_tool_call.""" if block_message is not None: - result = json.dumps({"error": block_message}, ensure_ascii=False) - error_type = block_error_type - error_message = block_message + result, error_type, error_message = json.dumps({"error": block_message}, ensure_ascii=False), block_error_type, block_message else: result = agent._guardrail_block_result(guardrail_decision) error_type = "guardrail_block" @@ -867,14 +847,12 @@ def _begin_tool_execution(agent, ref: _ToolCallRef, display_index: int | None) - function_name, function_args, effective_task_id, tool_call_id = ref.name, ref.args, ref.task_id, ref.call_id display_args = _redact_tool_args_for_display(function_name, function_args) or function_args if _tool_progress_enabled(agent): - args_str = json.dumps(display_args, ensure_ascii=False) prefix = f"Tool {display_index}" if display_index is not None else "Tool" if agent.verbose_logging: print(f" πŸ“ž {prefix}: {function_name}({list(display_args.keys())})") print(agent._wrap_verbose("Args: ", json.dumps(display_args, indent=2, ensure_ascii=False))) else: - args_preview = args_str[: agent.log_prefix_chars] + "..." if len(args_str) > agent.log_prefix_chars else args_str - print(f" πŸ“ž {prefix}: {function_name}({list(function_args.keys())}) - {args_preview}") + print(f" πŸ“ž {prefix}: {function_name}({list(function_args.keys())}) - {_preview(json.dumps(display_args, ensure_ascii=False), agent.log_prefix_chars)}") agent._current_tool = function_name agent._touch_activity(f"executing tool: {function_name}") @@ -891,7 +869,7 @@ def _begin_tool_execution(agent, ref: _ToolCallRef, display_index: int | None) - if not agent._checkpoint_mgr.enabled: return - try: + with contextlib.suppress(Exception): if function_name in {"write_file", "patch"}: _ensure_file_checkpoint(agent, function_name, function_args, effective_task_id) elif function_name == "terminal": @@ -899,8 +877,6 @@ def _begin_tool_execution(agent, ref: _ToolCallRef, display_index: int | None) - if _is_destructive_command(command): cwd = function_args.get("workdir") or os.getenv("TERMINAL_CWD", os.getcwd()) agent._checkpoint_mgr.ensure_checkpoint(cwd, f"before terminal: {command[:60]}") - except Exception: - pass def _emit_tool_complete_and_risk(agent, ref: _ToolCallRef, result, risk_metadata, blocked: bool) -> None: @@ -1017,15 +993,17 @@ def _tool_progress_enabled(agent) -> bool: return not agent.quiet_mode and getattr(agent, "tool_progress_mode", "all") != "off" +def _preview(text: str, limit: int) -> str: + return text[:limit] + "..." if len(text) > limit else text + + def _print_tool_completed(agent, index: int, tool_duration: float, result) -> None: """Non-quiet ``βœ… Tool N completed`` line (full result under verbose logging).""" if agent.verbose_logging: print(f" βœ… Tool {index} completed in {tool_duration:.2f}s") print(agent._wrap_verbose("Result: ", result)) else: - preview = result if isinstance(result, str) else str(result) - response_preview = preview[:agent.log_prefix_chars] + "..." if len(preview) > agent.log_prefix_chars else preview - print(f" βœ… Tool {index} completed in {tool_duration:.2f}s - {response_preview}") + print(f" βœ… Tool {index} completed in {tool_duration:.2f}s - {_preview(result if isinstance(result, str) else str(result), agent.log_prefix_chars)}") # ── Concurrent batch machinery ────────────────────────────────────────────── @@ -1153,10 +1131,8 @@ class _ConcurrentBatch: logger.info("tool %s abandoned at start-order gate; skipping dispatch", ref.name) return None except KeyboardInterrupt: - try: + with contextlib.suppress(Exception): agent.interrupt("keyboard interrupt") - except Exception: - pass result = ref.emit_cancelled(agent, start) duration = time.time() - start logger.info("tool %s cancelled (%.2fs)", ref.name, duration) @@ -1561,10 +1537,8 @@ def _run_sequential_call( if not dispatch.handles_keyboard_interrupt: raise _spinner_result = ref.emit_cancelled(agent, tool_start_time) - try: + with contextlib.suppress(Exception): agent.interrupt("keyboard interrupt") - except Exception: - pass _append_skipped_tool_results( agent, messages, remaining_calls, ref.task_id, content="[Tool execution cancelled β€” {name} was skipped due to keyboard interrupt]",