From f8c4ec8469193f1d759eebae47fe5119fd868faf Mon Sep 17 00:00:00 2001 From: Xipong <217837358+Xipong@users.noreply.github.com> Date: Wed, 29 Jul 2026 23:28:53 +0300 Subject: [PATCH 1/7] feat(delegation): add same-turn result injection --- agent/conversation_loop.py | 49 ++- agent/delegation_inject.py | 155 +++++++ gateway/run.py | 62 ++- run_agent.py | 11 +- tests/agent/test_delegation_inject.py | 360 ++++++++++++++++ .../test_delegate_apiserver_background.py | 63 +++ tools/async_delegation.py | 393 ++++++++++++++++-- tools/delegate_tool.py | 134 ++++-- tools/process_registry.py | 37 +- website/docs/guides/delegation-patterns.md | 10 +- .../docs/user-guide/features/delegation.md | 47 ++- 11 files changed, 1203 insertions(+), 118 deletions(-) create mode 100644 agent/delegation_inject.py create mode 100644 tests/agent/test_delegation_inject.py diff --git a/agent/conversation_loop.py b/agent/conversation_loop.py index 3ca96898cf99..eee7ecd2d4d9 100644 --- a/agent/conversation_loop.py +++ b/agent/conversation_loop.py @@ -1179,6 +1179,10 @@ def run_conversation( active_system_prompt = _ctx.active_system_prompt effective_task_id = _ctx.effective_task_id turn_id = _ctx.turn_id + # delegate_task snapshots this id at dispatch. A late inject result whose + # originating turn already ended must stay on the normal synthetic-turn + # path instead of leaking into a later user turn. + agent._active_turn_id = turn_id current_turn_user_idx = _ctx.current_turn_user_idx _should_review_memory = _ctx.should_review_memory _plugin_user_context = _ctx.plugin_user_context @@ -1256,6 +1260,16 @@ def run_conversation( ) while (api_call_count < agent.max_iterations and agent.iteration_budget.remaining > 0) or agent._budget_grace_call: + # Safe boundary: the previous assistant tool-call block (if any) and + # every corresponding tool result have already been appended. Drain + # only already-ready results from this foreground turn. + try: + from agent.delegation_inject import drain_ready_injects + + drain_ready_injects(agent, messages, turn_id) + except Exception: + logger.debug("Same-turn delegation inject drain failed", exc_info=True) + _redirect_text = agent._drain_pending_redirect() if _redirect_text: _apply_active_turn_redirect(agent, messages, _redirect_text) @@ -6304,6 +6318,16 @@ def _perform_api_call(next_api_kwargs): # gateway kills the session before the next activity # touch fires (#69559, #69131). agent._touch_activity(f"tool results posted, continuing iteration #{api_call_count}") + # Safe boundary even when this was the nominal last iteration: + # every tool result is present and post-tool compression is done. + # Drain once without waiting; a ready inject grants exactly one + # reconciliation call only if the normal budget is exhausted. + try: + from agent.delegation_inject import drain_ready_injects + + drain_ready_injects(agent, messages, turn_id) + except Exception: + logger.debug("Post-tool delegation inject drain failed", exc_info=True) # Continue loop for next response continue @@ -6911,8 +6935,28 @@ def _perform_api_call(next_api_kwargs): final_response = None continue - messages.append(final_msg) - + # Treat this answer as provisional until one final + # non-blocking inject drain has run. If an auditor/dependency + # completed at this boundary, append its report after the + # provisional assistant message and reconcile once more. + try: + from agent.delegation_inject import reconcile_provisional_final + + _ready_injects = reconcile_provisional_final( + agent, messages, final_msg, turn_id=turn_id + ) + except Exception: + logger.debug("Final delegation inject drain failed", exc_info=True) + # Preserve the original final even if the optional drain + # machinery itself fails before appending it. + if not messages or messages[-1] is not final_msg: + messages.append(final_msg) + _ready_injects = False + if _ready_injects: + final_response = None + agent._session_messages = messages + continue + _turn_exit_reason = f"text_response(finish_reason={finish_reason})" if not agent.quiet_mode: agent._safe_print(f"πŸŽ‰ Conversation completed after {api_call_count} OpenAI-compatible API call(s)") @@ -7017,6 +7061,7 @@ def _perform_api_call(next_api_kwargs): # (god-file decomposition Phase 1 step 4). Behavior-neutral: the assembled # result dict is returned exactly as before. from agent.turn_finalizer import finalize_turn + return finalize_turn( agent, final_response=final_response, diff --git a/agent/delegation_inject.py b/agent/delegation_inject.py new file mode 100644 index 000000000000..ff1402b30e25 --- /dev/null +++ b/agent/delegation_inject.py @@ -0,0 +1,155 @@ +"""Same-turn delivery of completed background delegations. + +The async registry and its durable claims remain the authority for delivery. +This module only drains already-ready ``result_delivery=inject`` events at +conversation-loop safe boundaries. It never waits for a child. +""" + +from __future__ import annotations + +import logging +import os +import queue +from typing import Any + +logger = logging.getLogger(__name__) + + +def _grant_reconciliation_grace_if_needed(agent: Any) -> None: + """Allow one request only when the normal loop budget is exhausted.""" + + api_calls = int(getattr(agent, "_api_call_count", 0) or 0) + max_iterations = int(getattr(agent, "max_iterations", 0) or 0) + budget = getattr(agent, "iteration_budget", None) + remaining = int(getattr(budget, "remaining", 0) or 0) + if (max_iterations and api_calls >= max_iterations) or remaining <= 0: + agent._budget_grace_call = True + + +def reconcile_provisional_final( + agent: Any, + messages: list[dict[str, Any]], + final_message: dict[str, Any], + *, + turn_id: str, +) -> bool: + """Append a provisional assistant final and reconcile ready injects. + + Returns ``True`` only when at least one result was appended after the + assistant message. The caller must then continue the normal model loop + instead of committing the provisional final. The message object is never + mutated after append, preserving prompt-cache history. + """ + + messages.append(final_message) + ready = drain_ready_injects(agent, messages, turn_id=turn_id) + if not ready: + return False + # One reconciliation request must remain possible if the provisional final + # consumed the nominal iteration budget. This grants no waiting and does not + # bypass a budget that still has room. + _grant_reconciliation_grace_if_needed(agent) + return True + + +def drain_ready_injects(agent: Any, messages: list[dict[str, Any]], turn_id: str) -> int: + """Append one grouped synthetic user message for ready events from *turn_id*. + + The queue is scanned once using its size at entry. Non-matching events are + requeued in their original order, so terminal/watch notifications and + ``after_turn`` delegation results are left to their existing consumers. + Returns the number of delegation results appended. No blocking calls are + made. + """ + + if not turn_id: + return 0 + # A previous inject may already be the tail while its reconciliation API + # request is being retried after a transport failure. Appending another + # synthetic user message here would create userβ†’user history and force the + # sequence repairer to rewrite cached context. Leave all events queued until + # an assistant response establishes the next append-only boundary. + if messages and messages[-1].get("role") == "user": + return 0 + + from tools.async_delegation import ( + claim_event_delivery, + complete_event_delivery, + release_event_delivery, + ) + from tools.process_registry import _format_async_delegation, process_registry + + completion_queue = process_registry.completion_queue + try: + scan_count = completion_queue.qsize() + except Exception: + return 0 + + accepted: list[tuple[dict[str, Any], str, str]] = [] + for _ in range(max(0, scan_count)): + try: + event = completion_queue.get_nowait() + except queue.Empty: + break + except Exception: + break + + delivery = str(event.get("result_delivery") or "after_turn").strip().lower() + event_turn_id = str(event.get("parent_turn_id") or "") + if ( + event.get("type") != "async_delegation" + or delivery != "inject" + or event_turn_id != str(turn_id) + ): + completion_queue.put(event) + continue + + claim_id = claim_event_delivery(event, f"conversation-loop:{os.getpid()}") + if claim_id is None: + # A competing CLI/gateway process already owns this durable event, + # or it was delivered from a duplicate restored queue entry. + continue + + try: + text = _format_async_delegation(event) + except Exception: + logger.debug("Failed to format inject delegation event", exc_info=True) + release_event_delivery(event, claim_id) + completion_queue.put(event) + continue + if not text: + release_event_delivery(event, claim_id) + completion_queue.put(event) + continue + accepted.append((event, claim_id, text)) + + if not accepted: + return 0 + + content = "\n\n".join(item[2] for item in accepted) + delegation_ids = [str(item[0].get("delegation_id") or "") for item in accepted] + event_ids = [ + f"{item[0].get('delegation_id') or ''}:{item[0].get('delivery_event_key') or 'aggregate'}" + for item in accepted + ] + try: + messages.append( + { + "role": "user", + "content": content, + "_synthetic_delegation_inject": True, + "_delegation_ids": delegation_ids, + "_delegation_event_ids": event_ids, + } + ) + agent._session_messages = messages + except Exception: + for event, claim_id, _text in accepted: + release_event_delivery(event, claim_id) + completion_queue.put(event) + raise + + for event, claim_id, _text in accepted: + complete_event_delivery(event, claim_id) + _grant_reconciliation_grace_if_needed(agent) + return len(accepted) diff --git a/gateway/run.py b/gateway/run.py index 911021a36ee6..10b0fbf36335 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -20934,7 +20934,8 @@ def _completion_delivery_identity(evt: dict) -> Optional[tuple[str, str, object] evt_type = str(evt.get("type") or "") if evt_type == "async_delegation": producer_id = str(evt.get("delegation_id") or "") - return (evt_type, producer_id, "") if producer_id else None + event_key = str(evt.get("delivery_event_key") or "") + return (evt_type, producer_id, event_key) if producer_id else None if evt_type == "completion": producer_id = str(evt.get("session_id") or "") started_at = evt.get("started_at") @@ -21013,12 +21014,12 @@ async def _deliver_completion_notification( durable_delegation_id = str(evt.get("delegation_id") or "") if durable_delegation_id: try: - from tools.async_delegation import claim_completion_delivery + from tools.async_delegation import claim_event_delivery - durable_claim_id = f"gateway:{id(self)}:{__import__('uuid').uuid4().hex}" - if not claim_completion_delivery( - durable_delegation_id, durable_claim_id, - ): + durable_claim_id = claim_event_delivery( + evt, f"gateway:{id(self)}" + ) or "" + if not durable_claim_id: return None except Exception as exc: logger.warning( @@ -21044,11 +21045,9 @@ async def _deliver_completion_notification( ) if durable_claim_id: try: - from tools.async_delegation import drop_completion_delivery + from tools.async_delegation import drop_event_delivery - drop_completion_delivery( - durable_delegation_id, durable_claim_id, - ) + drop_event_delivery(evt, durable_claim_id) except Exception: logger.debug( "Could not drop durable completion claim", @@ -21058,11 +21057,9 @@ async def _deliver_completion_notification( if verdict == "retry": if durable_claim_id: try: - from tools.async_delegation import release_completion_delivery + from tools.async_delegation import release_event_delivery - release_completion_delivery( - durable_delegation_id, durable_claim_id, - ) + release_event_delivery(evt, durable_claim_id) except Exception: logger.debug( "Could not release durable completion claim", @@ -21100,11 +21097,9 @@ async def _deliver_completion_notification( # after adapter acceptance; this gateway keeps no parallel ledger. if durable_claim_id: try: - from tools.async_delegation import complete_completion_delivery + from tools.async_delegation import complete_event_delivery - complete_completion_delivery( - durable_delegation_id, durable_claim_id, - ) + complete_event_delivery(evt, durable_claim_id) except Exception as exc: logger.warning( "Could not acknowledge durable async completion %s: %s", @@ -21117,11 +21112,9 @@ async def _deliver_completion_notification( self._completion_deliveries_inflight.discard(identity) if durable_claim_id and not accepted: try: - from tools.async_delegation import release_completion_delivery + from tools.async_delegation import release_event_delivery - release_completion_delivery( - durable_delegation_id, durable_claim_id, - ) + release_event_delivery(evt, durable_claim_id) except Exception: logger.debug("Could not release durable completion claim", exc_info=True) @@ -21182,6 +21175,31 @@ async def _async_delegation_watcher(self, interval: float = 2.0) -> None: _pr.completion_queue.put(evt) for evt in async_events: self._enrich_async_delegation_routing(evt) + # Gateway busy-session deferral for 'inject' events: + # if the parent session is currently running another + # turn, leave the inject queued so the conversation + # loop's safe-boundary drain can pick it up when the + # turn finishes. This keeps inject from being claimed + # away from the active parent. 'after_turn' events are + # always delivered here (the legacy path). + _rd = str(evt.get("result_delivery") or "after_turn").strip().lower() + if _rd == "inject": + _route_key = str(evt.get("session_key") or "").strip() + _event_turn_id = str(evt.get("parent_turn_id") or "") + _running_parent = self._running_agents.get(_route_key) + if _running_parent is _AGENT_PENDING_SENTINEL: + _pr.completion_queue.put(evt) + continue + if ( + _running_parent is not None + and _event_turn_id + and str( + getattr(_running_parent, "_active_turn_id", "") or "" + ) + == _event_turn_id + ): + _pr.completion_queue.put(evt) + continue synth_text = _format_gateway_process_notification(evt) if not synth_text: continue diff --git a/run_agent.py b/run_agent.py index 54cfb18e9796..2b7b42274264 100644 --- a/run_agent.py +++ b/run_agent.py @@ -6897,12 +6897,10 @@ def _dispatch_delegate_task(self, function_args: dict) -> str: _strip_model_hidden_task_fields, delegate_task as _delegate_task, ) - # Delegations from the top-level MODEL always run in the background β€” - # the model does not get to choose. delegate_task returns immediately - # with a handle (one per task) and each subagent's result re-enters the - # conversation as a new message when it finishes. This applies to BOTH - # a single task and a fan-out batch (each task becomes its own - # independent background subagent). The one exception: + # Delegations from the top-level model always run in the background β€” + # the model chooses only result delivery. ``after_turn`` keeps the legacy + # synthetic-turn route; ``inject`` reconciles already-ready results at + # safe boundaries in this turn. The one exception: # - A delegation from an ORCHESTRATOR SUBAGENT (depth > 0) stays # synchronous: the orchestrator needs its workers' results within # its own turn to compose a summary, and a subagent doesn't own the @@ -6916,6 +6914,7 @@ def _dispatch_delegate_task(self, function_args: dict) -> str: max_iterations=function_args.get("max_iterations"), role=function_args.get("role"), background=(not _is_subagent), + result_delivery=function_args.get("result_delivery"), parent_agent=self, ) diff --git a/tests/agent/test_delegation_inject.py b/tests/agent/test_delegation_inject.py new file mode 100644 index 000000000000..fb6bd8e901e0 --- /dev/null +++ b/tests/agent/test_delegation_inject.py @@ -0,0 +1,360 @@ +"""Behavioral contracts for same-turn async delegation injection.""" + +from __future__ import annotations + +from copy import deepcopy +from types import SimpleNamespace +import threading +import time +import uuid + +import pytest + +from agent.delegation_inject import drain_ready_injects, reconcile_provisional_final +from tools import async_delegation as ad +from tools import delegate_tool +from tools.process_registry import process_registry + + +@pytest.fixture(autouse=True) +def _clean_async_state(): + ad._reset_for_tests() + while not process_registry.completion_queue.empty(): + process_registry.completion_queue.get_nowait() + yield + deadline = time.monotonic() + 2 + while ad.active_count() and time.monotonic() < deadline: + time.sleep(0.01) + ad._reset_for_tests() + while not process_registry.completion_queue.empty(): + process_registry.completion_queue.get_nowait() + + +def _record(*, goals=("audit",), turn_id="turn-current", delivery="inject"): + delegation_id = f"deleg_test_{uuid.uuid4().hex}" + record = { + "delegation_id": delegation_id, + "goal": goals[0] if len(goals) == 1 else f"{len(goals)} tasks", + "goals": list(goals), + "context": "parent context", + "toolsets": ["file"], + "role": "leaf", + "model": "child-model", + "session_key": "agent:main:cli:dm:local", + "origin_ui_session_id": "", + "origin_session_id": "", + "parent_session_id": "parent-session", + "parent_turn_id": turn_id, + "status": "running", + "dispatched_at": time.time(), + "completed_at": None, + "is_batch": True, + "result_delivery": delivery, + } + with ad._records_lock: + ad._records[delegation_id] = record + ad._persist_dispatch(record) + return delegation_id + + +def _child(index: int, summary: str, *, status="completed", error=None): + return { + "task_index": index, + "status": status, + "summary": summary, + "error": error, + "api_calls": 2, + "duration_seconds": 0.25, + } + + +def _queue_contents(): + items = [] + while not process_registry.completion_queue.empty(): + items.append(process_registry.completion_queue.get_nowait()) + for item in items: + process_registry.completion_queue.put(item) + return items + + +def _event_state(delegation_id: str, event_key: str): + with ad._DB_LOCK, ad._transaction() as conn: + return conn.execute( + "SELECT delivery_state, delivery_attempts FROM async_delegation_events " + "WHERE delegation_id=? AND event_key=?", + (delegation_id, event_key), + ).fetchone() + + +def test_inject_drain_rotates_unrelated_queue_items_and_coalesces_ready_children(): + delegation_id = _record(goals=("audit A", "audit B")) + unrelated = {"type": "completion", "session_id": "process-1"} + legacy = { + "type": "async_delegation", + "delegation_id": "legacy-after-turn", + "result_delivery": "after_turn", + } + process_registry.completion_queue.put(unrelated) + assert ad.publish_batch_child_completion(delegation_id, 0, _child(0, "finding A")) + assert ad.publish_batch_child_completion(delegation_id, 1, _child(1, "finding B")) + process_registry.completion_queue.put(legacy) + + messages = [ + {"role": "assistant", "tool_calls": [{"id": "tc1", "function": {}}]}, + {"role": "tool", "tool_call_id": "tc1", "content": "done"}, + ] + count = drain_ready_injects( + SimpleNamespace(_active_turn_id="turn-current"), messages, "turn-current" + ) + + assert count == 2 + assert [m["role"] for m in messages] == ["assistant", "tool", "user"] + assert "finding A" in messages[-1]["content"] + assert "finding B" in messages[-1]["content"] + assert "TASK 1/2" in messages[-1]["content"] + assert "TASK 2/2" in messages[-1]["content"] + assert messages[-1]["_delegation_event_ids"] == [ + f"{delegation_id}:task:0", + f"{delegation_id}:task:1", + ] + assert _queue_contents() == [unrelated, legacy] + assert _event_state(delegation_id, "task:0") == ("delivered", 1) + assert _event_state(delegation_id, "task:1") == ("delivered", 1) + + +def test_inject_drain_is_exactly_once_when_queue_contains_duplicate_event(): + delegation_id = _record() + assert ad.publish_batch_child_completion(delegation_id, 0, _child(0, "once")) + event = process_registry.completion_queue.get_nowait() + process_registry.completion_queue.put(event) + process_registry.completion_queue.put(deepcopy(event)) + messages = [{"role": "tool", "tool_call_id": "tc", "content": "done"}] + + assert drain_ready_injects(SimpleNamespace(), messages, "turn-current") == 1 + assert messages[-1]["content"].count("once") == 1 + assert process_registry.completion_queue.empty() + assert drain_ready_injects(SimpleNamespace(), messages, "turn-current") == 0 + assert _event_state(delegation_id, "task:0") == ("delivered", 1) + + +def test_wrong_turn_and_missing_mode_are_not_injected(): + delegation_id = _record(turn_id="old-turn") + assert ad.publish_batch_child_completion(delegation_id, 0, _child(0, "late")) + legacy = { + "type": "async_delegation", + "delegation_id": "legacy", + "parent_turn_id": "turn-current", + } + process_registry.completion_queue.put(legacy) + messages = [{"role": "tool", "tool_call_id": "tc", "content": "done"}] + + assert drain_ready_injects(SimpleNamespace(), messages, "turn-current") == 0 + assert messages == [{"role": "tool", "tool_call_id": "tc", "content": "done"}] + assert [e.get("delegation_id") for e in _queue_contents()] == [ + delegation_id, + "legacy", + ] + assert _event_state(delegation_id, "task:0") == ("pending", 0) + + +def test_provisional_final_is_append_only_and_gets_budgeted_reconciliation(): + delegation_id = _record() + assert ad.publish_batch_child_completion( + delegation_id, 0, _child(0, "review changed the answer") + ) + agent = SimpleNamespace( + _active_turn_id="turn-current", + _budget_grace_call=False, + _api_call_count=1, + max_iterations=1, + iteration_budget=SimpleNamespace(remaining=0), + ) + final_message = {"role": "assistant", "content": "provisional answer"} + final_snapshot = deepcopy(final_message) + messages = [{"role": "tool", "tool_call_id": "tc", "content": "done"}] + + assert reconcile_provisional_final( + agent, messages, final_message, turn_id="turn-current" + ) is True + assert [message["role"] for message in messages] == ["tool", "assistant", "user"] + assert messages[-2] is final_message + assert final_message == final_snapshot + assert "review changed the answer" in messages[-1]["content"] + assert agent._budget_grace_call is True + + +def test_ready_inject_uses_normal_budget_when_available(): + delegation_id = _record() + assert ad.publish_batch_child_completion( + delegation_id, 0, _child(0, "ready") + ) + agent = SimpleNamespace( + _active_turn_id="turn-current", + _budget_grace_call=False, + _api_call_count=1, + max_iterations=5, + iteration_budget=SimpleNamespace(remaining=4), + ) + messages = [{"role": "assistant", "content": "provisional"}] + assert drain_ready_injects(agent, messages, "turn-current") == 1 + assert agent._budget_grace_call is False + + +def test_retry_tail_user_defers_new_inject_without_rewriting_history(): + delegation_id = _record(goals=("first", "second")) + assert ad.publish_batch_child_completion( + delegation_id, 0, _child(0, "first result") + ) + agent = SimpleNamespace(_active_turn_id="turn-current") + messages = [{"role": "assistant", "content": "working"}] + assert drain_ready_injects(agent, messages, "turn-current") == 1 + snapshot = deepcopy(messages) + + assert ad.publish_batch_child_completion( + delegation_id, 1, _child(1, "second result") + ) + assert drain_ready_injects(agent, messages, "turn-current") == 0 + assert messages == snapshot + assert [event["delivery_event_key"] for event in _queue_contents()] == ["task:1"] + + +def test_inject_batch_publishes_first_child_without_waiting_for_sibling(): + gate = threading.Event() + ready = threading.Event() + delegation_id = f"deleg_partial_{uuid.uuid4().hex}" + + def runner(): + assert ad.publish_batch_child_completion( + delegation_id, 0, _child(0, "fast result") + ) + ready.set() + gate.wait(timeout=5) + return { + "results": [_child(0, "fast result"), _child(1, "slow result")], + "total_duration_seconds": 0.5, + } + + dispatched = ad.dispatch_async_delegation_batch( + goals=["fast", "slow"], + context=None, + toolsets=None, + role="leaf", + model="m", + session_key="agent:main:cli:dm:local", + parent_session_id="parent", + parent_turn_id="turn-current", + runner=runner, + max_async_children=1, + delegation_id=delegation_id, + result_delivery="inject", + ) + assert dispatched["status"] == "dispatched" + assert ready.wait(timeout=2) + first = process_registry.completion_queue.get(timeout=2) + assert first["delivery_event_key"] == "task:0" + assert first["results"][0]["summary"] == "fast result" + assert ad.get_durable_delegation(delegation_id)["state"] == "running" + + gate.set() + deadline = time.monotonic() + 3 + second = None + while time.monotonic() < deadline: + try: + candidate = process_registry.completion_queue.get(timeout=0.05) + except Exception: + continue + if candidate.get("delivery_event_key") == "task:1": + second = candidate + break + assert second is not None + assert second["results"][0]["summary"] == "slow result" + assert "delivery_event_key" not in { + e.get("delivery_event_key") for e in _queue_contents() + } + deadline = time.monotonic() + 2 + while ad.active_count() and time.monotonic() < deadline: + time.sleep(0.01) + assert ad.get_durable_delegation(delegation_id)["delivery_state"] == "delivered" + + +def test_after_turn_remains_default_and_emits_only_combined_batch_event(): + gate = threading.Event() + + def runner(): + gate.wait(timeout=5) + return { + "results": [_child(0, "A"), _child(1, "B")], + "total_duration_seconds": 0.1, + } + + dispatched = ad.dispatch_async_delegation_batch( + goals=["A", "B"], context=None, toolsets=None, role="leaf", model="m", + session_key="", runner=runner, max_async_children=1, + ) + assert process_registry.completion_queue.empty() + gate.set() + event = process_registry.completion_queue.get(timeout=3) + assert event["delegation_id"] == dispatched["delegation_id"] + assert event["result_delivery"] == "after_turn" + assert "delivery_event_key" not in event + assert [r["summary"] for r in event["results"]] == ["A", "B"] + + +def test_child_timeout_error_is_injectable_and_durable(): + delegation_id = _record() + assert ad.publish_batch_child_completion( + delegation_id, + 0, + _child(0, "", status="timeout", error="child exceeded 10s"), + ) + messages = [{"role": "tool", "tool_call_id": "tc", "content": "done"}] + assert drain_ready_injects(SimpleNamespace(), messages, "turn-current") == 1 + assert "timeout" in messages[-1]["content"] + assert "child exceeded 10s" in messages[-1]["content"] + assert _event_state(delegation_id, "task:0") == ("delivered", 1) + + +def test_pending_child_event_restores_with_same_durable_identity(tmp_path, monkeypatch): + monkeypatch.setattr(ad, "_db_path", lambda: tmp_path / "state.db") + delegation_id = _record(goals=("restore me",)) + assert ad.publish_batch_child_completion( + delegation_id, 0, _child(0, "restored result") + ) + process_registry.completion_queue.get_nowait() + + restored_queue = __import__("queue").Queue() + assert ad.restore_undelivered_completions(restored_queue) == 1 + event = restored_queue.get_nowait() + assert event["restored"] is True + assert event["delegation_id"] == delegation_id + assert event["delivery_event_key"] == "task:0" + + claim = ad.claim_event_delivery(event, "restart-consumer") + assert claim + ad.complete_event_delivery(event, claim) + assert ad.restore_undelivered_completions(restored_queue) == 0 + + +def test_model_schema_defaults_after_turn_and_dispatch_forwards_explicit_mode(monkeypatch): + delivery_schema = delegate_tool.DELEGATE_TASK_SCHEMA["parameters"]["properties"][ + "result_delivery" + ] + assert delivery_schema["enum"] == ["inject", "after_turn"] + assert delivery_schema["default"] == "after_turn" + + captured = {} + + def fake_delegate_task(**kwargs): + captured.update(kwargs) + return "ok" + + monkeypatch.setattr(delegate_tool, "delegate_task", fake_delegate_task) + from run_agent import AIAgent + + result = AIAgent._dispatch_delegate_task( + SimpleNamespace(_delegate_depth=0), + {"goal": "audit", "result_delivery": "inject"}, + ) + assert result == "ok" + assert captured["background"] is True + assert captured["result_delivery"] == "inject" diff --git a/tests/tools/test_delegate_apiserver_background.py b/tests/tools/test_delegate_apiserver_background.py index 4c33cf9153bb..e5fc3d8d3bc7 100644 --- a/tests/tools/test_delegate_apiserver_background.py +++ b/tests/tools/test_delegate_apiserver_background.py @@ -107,6 +107,69 @@ def clobbering_build_child(**kw): return dt +def test_inject_batch_emits_fast_child_before_slow_child(monkeypatch): + """Exercise delegate_tool's real per-child callback seam, not just ledger APIs.""" + dt = _patch_delegate(monkeypatch) + slow_gate = __import__("threading").Event() + + def staggered_child(task_index, goal, child=None, parent_agent=None, **kw): + if task_index == 1: + slow_gate.wait(timeout=5) + return { + "task_index": task_index, + "status": "completed", + "summary": f"done: {goal}", + "api_calls": 1, + "duration_seconds": 0.1, + "model": "m", + "exit_reason": "completed", + } + + monkeypatch.setattr(dt, "_run_single_child", staggered_child) + set_session_vars( + platform="telegram", + chat_id="7", + session_key="agent:main:telegram:dm:7", + session_id="parent-sess", + async_delivery=True, + ) + parent = _fake_parent() + parent._active_turn_id = "turn-live" + parsed = json.loads( + dt.delegate_task( + tasks=[{"goal": "fast"}, {"goal": "slow"}], + background=True, + result_delivery="inject", + parent_agent=parent, + ) + ) + assert parsed["status"] == "dispatched", parsed + + first = _drain_one(timeout=2) + assert first is not None + assert first["delivery_event_key"] == "task:0" + assert first["parent_turn_id"] == "turn-live" + assert first["results"][0]["summary"] == "done: fast" + import tools.async_delegation as ad + + claim = ad.claim_event_delivery(first, "test-consumer") + assert claim + ad.complete_event_delivery(first, claim) + + slow_gate.set() + second = _drain_one(timeout=2) + assert second is not None + assert second["delivery_event_key"] == "task:1" + assert second["results"][0]["summary"] == "done: slow" + claim = ad.claim_event_delivery(second, "test-consumer") + assert claim + ad.complete_event_delivery(second, claim) + deadline = time.time() + 2 + while ad.active_count() and time.time() < deadline: + time.sleep(0.01) + assert ad.active_count() == 0 + + def test_apiserver_session_with_id_dispatches_background(monkeypatch): """async_delivery=False + a raw session id (HERMES_SESSION_ID) β†’ background dispatch (the completion wakes the session via the diff --git a/tools/async_delegation.py b/tools/async_delegation.py index 702036df0c41..5572f5151d2a 100644 --- a/tools/async_delegation.py +++ b/tools/async_delegation.py @@ -6,27 +6,28 @@ subagent that runs on a module-level daemon executor and returns a handle immediately, so the user and the model can keep working while the child runs. -When the child finishes, a completion event is pushed onto the SHARED -``process_registry.completion_queue`` with ``type="async_delegation"``. The -CLI (``cli.py`` process_loop) and gateway (``_run_process_watcher`` / -``completion_queue`` drain) already poll that queue while the agent is idle -and forge a fresh user/internal turn from each event. We deliberately reuse -that rail rather than reaching into a running agent loop: - - - completions surface as a NEW turn when the agent is idle, never spliced - between a tool result and an assistant message. That keeps strict - message-role alternation legal and the prompt cache intact (hard - invariant: never mutate past context). - - we inherit the queue's de-dup, crash-recovery checkpoint, and the - existing CLI + gateway drain wiring for free β€” no new drain loops in the - two largest files in the repo. - -The completion payload carries a RICH, self-contained task-source block (the +When a child finishes, its completion event is written to the durable ledger +and pushed onto the shared ``process_registry.completion_queue``. The event's +``result_delivery`` selects one of two consumers without creating a parallel +execution system: + + - ``after_turn`` (default) keeps the existing CLI/gateway synthetic-turn + path. A batch publishes one consolidated result after all children finish. + - ``inject`` lets the originating conversation loop claim already-ready + events at append-only safe boundaries, after a complete tool-result block + and before the next model request. Batch children publish independently as + they finish. A result that misses its originating turn remains on the same + queue and follows the normal synthetic-turn path. + +Both paths use the same durable claim/recovery ledger. Neither waits or polls +for a running child, past history is never rewritten, and competing consumers +cannot acknowledge the same event twice. + +The completion payload carries a rich, self-contained task-source block (the original goal, the context the parent supplied, toolsets, model, dispatch -time, status, and the full result summary). When the result re-enters the -conversation the parent may be deep in unrelated context and won't remember -why the subagent existed; the block lets it either use the result or -re-dispatch if the world has moved on. +time, status, and the full result summary). When a result arrives after the +originating turn, that block lets the parent use it or re-dispatch if the world +has moved on. This module owns ONLY the async lifecycle. The actual child build + run is delegated back to ``delegate_tool._run_single_child`` via an injected @@ -173,9 +174,31 @@ def _initialize_schema(conn: sqlite3.Connection) -> None: # completions recovered after a process restart are unroutable on # api_server (the in-memory record that carried it is gone). ("origin_session_id", "TEXT"), + # Delivery mode captured at dispatch: 'inject' delivers at safe + # conversation-loop boundaries; 'after_turn' preserves the legacy + # synthetic-turn path. Missing/invalid values mean 'after_turn'. + ("result_delivery", "TEXT"), ): if name not in columns: conn.execute(f"ALTER TABLE async_delegations ADD COLUMN {name} {sql_type}") + # Child-level delivery records extend the existing durable claim ledger. + # A batch parent remains one execution unit, while each completed child is + # independently claimable/recoverable for result_delivery=inject. + conn.execute( + """CREATE TABLE IF NOT EXISTS async_delegation_events ( + delegation_id TEXT NOT NULL, + event_key TEXT NOT NULL, + event_json TEXT NOT NULL, + delivery_state TEXT NOT NULL DEFAULT 'pending', + delivery_attempts INTEGER NOT NULL DEFAULT 0, + delivered_at REAL, + delivery_claim TEXT, + delivery_claimed_at REAL, + created_at REAL NOT NULL, + updated_at REAL NOT NULL, + PRIMARY KEY (delegation_id, event_key) + )""" + ) @contextmanager @@ -206,28 +229,45 @@ def _persist_dispatch(record: Dict[str, Any]) -> None: owner_started_at = None task_payload = { key: record.get(key) - for key in ("goal", "goals", "context", "toolsets", "role", "model", "is_batch") + for key in ( + "goal", + "goals", + "context", + "toolsets", + "role", + "model", + "is_batch", + "result_delivery", + "parent_turn_id", + ) if key in record } + _result_delivery = str(record.get("result_delivery") or "after_turn").strip().lower() + if _result_delivery not in {"inject", "after_turn"}: + _result_delivery = "after_turn" with _DB_LOCK, _transaction() as conn: conn.execute( """INSERT OR REPLACE INTO async_delegations (delegation_id, origin_session, origin_ui_session_id, parent_session_id, state, dispatched_at, updated_at, delivery_state, delivery_attempts, owner_pid, - owner_started_at, task_json, origin_session_id) - VALUES (?, ?, ?, ?, 'running', ?, ?, 'pending', 0, ?, ?, ?, ?)""", + owner_started_at, task_json, origin_session_id, result_delivery) + VALUES (?, ?, ?, ?, 'running', ?, ?, 'pending', 0, ?, ?, ?, ?, ?)""", (record["delegation_id"], record.get("session_key", ""), record.get("origin_ui_session_id", ""), record.get("parent_session_id"), record["dispatched_at"], now, __import__("os").getpid(), owner_started_at, json.dumps(task_payload), - record.get("origin_session_id", "")), + record.get("origin_session_id", ""), _result_delivery), ) _prune_durable_records() def _delete_durable_delegation(delegation_id: str) -> None: with _DB_LOCK, _transaction() as conn: + conn.execute( + "DELETE FROM async_delegation_events WHERE delegation_id=?", + (delegation_id,), + ) conn.execute("DELETE FROM async_delegations WHERE delegation_id=?", (delegation_id,)) @@ -268,6 +308,12 @@ def _prune_durable_records() -> None: )""", (overflow,), ) + conn.execute( + """DELETE FROM async_delegation_events + WHERE delegation_id NOT IN ( + SELECT delegation_id FROM async_delegations + )""" + ) def _persist_completion(event: Dict[str, Any], result: Dict[str, Any]) -> None: @@ -282,6 +328,136 @@ def _persist_completion(event: Dict[str, Any], result: Dict[str, Any]) -> None: ) +def publish_batch_child_completion( + delegation_id: str, + task_index: int, + result: Dict[str, Any], +) -> bool: + """Durably enqueue one ready child from an ``inject`` batch. + + The parent batch remains the execution/stall unit. This function only + creates an independently claimable delivery event, keyed by task index. + Repeated publication is idempotent and never resets a delivered claim. + """ + with _records_lock: + record = dict(_records.get(delegation_id) or {}) + if str(record.get("result_delivery") or "after_turn").lower() != "inject": + return False + + goals = list(record.get("goals") or []) + goal = goals[task_index] if 0 <= task_index < len(goals) else record.get("goal", "") + completed_at = time.time() + child_result = dict(result or {}) + child_result.setdefault("task_index", task_index) + event_key = f"task:{task_index}" + event = { + "type": "async_delegation", + "delegation_id": delegation_id, + "delivery_event_key": event_key, + "batch_id": delegation_id, + "task_index": task_index, + "batch_size": len(goals), + "session_key": record.get("session_key", ""), + "origin_ui_session_id": record.get("origin_ui_session_id", ""), + "origin_session_id": record.get("origin_session_id", ""), + "parent_session_id": record.get("parent_session_id"), + "parent_turn_id": record.get("parent_turn_id", ""), + "goal": goal, + "goals": goals, + "context": record.get("context"), + "toolsets": record.get("toolsets"), + "role": record.get("role"), + "model": record.get("model"), + "status": child_result.get("status", "completed"), + "summary": child_result.get("summary"), + "error": child_result.get("error"), + "api_calls": child_result.get("api_calls"), + "duration_seconds": child_result.get("duration_seconds"), + "is_batch": True, + "results": [child_result], + "live_transcripts": ( + [child_result.get("live_transcript")] + if child_result.get("live_transcript") + else None + ), + "dispatched_at": record.get("dispatched_at"), + "completed_at": completed_at, + "result_delivery": "inject", + } + now = time.time() + with _DB_LOCK, _transaction() as conn: + cur = conn.execute( + """INSERT OR IGNORE INTO async_delegation_events + (delegation_id, event_key, event_json, delivery_state, + delivery_attempts, created_at, updated_at) + VALUES (?, ?, ?, 'pending', 0, ?, ?)""", + (delegation_id, event_key, json.dumps(event), now, now), + ) + if cur.rowcount != 1: + return False + from tools.process_registry import process_registry + + process_registry.completion_queue.put(event) + return True + + +def _publish_batch_terminal_event( + event_record: Dict[str, Any], combined: Dict[str, Any], status: str +) -> bool: + """Deliver a parent-level inject failure not represented by child events.""" + if status in {"completed", "success"} or (combined.get("results") or []): + return False + delegation_id = str(event_record.get("delegation_id") or "") + event_key = "terminal" + now = time.time() + event = { + "type": "async_delegation", + "delegation_id": delegation_id, + "delivery_event_key": event_key, + "batch_id": delegation_id, + "session_key": event_record.get("session_key", ""), + "origin_ui_session_id": event_record.get("origin_ui_session_id", ""), + "origin_session_id": event_record.get("origin_session_id", ""), + "parent_session_id": event_record.get("parent_session_id"), + "parent_turn_id": event_record.get("parent_turn_id", ""), + "goal": event_record.get("goal", ""), + "goals": event_record.get("goals"), + "context": event_record.get("context"), + "toolsets": event_record.get("toolsets"), + "role": event_record.get("role"), + "model": event_record.get("model"), + "status": status, + "is_batch": True, + "results": [], + "error": combined.get("error"), + "dispatched_at": event_record.get("dispatched_at"), + "completed_at": now, + "result_delivery": "inject", + } + for key in ( + "stalled_after_quiet_seconds", + "stall_threshold_seconds", + "stall_phase", + "stall_grace_seconds", + ): + if key in combined: + event[key] = combined[key] + with _DB_LOCK, _transaction() as conn: + cur = conn.execute( + """INSERT OR IGNORE INTO async_delegation_events + (delegation_id, event_key, event_json, delivery_state, + delivery_attempts, created_at, updated_at) + VALUES (?, ?, ?, 'pending', 0, ?, ?)""", + (delegation_id, event_key, json.dumps(event), now, now), + ) + if cur.rowcount != 1: + return False + from tools.process_registry import process_registry + + process_registry.completion_queue.put(event) + return True + + def _note_delivery_attempt(delegation_id: str) -> None: with _DB_LOCK, _transaction() as conn: conn.execute( @@ -302,12 +478,13 @@ def recover_abandoned_delegations() -> int: rows = conn.execute( """SELECT delegation_id, origin_session, origin_ui_session_id, parent_session_id, dispatched_at, owner_pid, - owner_started_at, task_json, origin_session_id + owner_started_at, task_json, origin_session_id, + result_delivery FROM async_delegations WHERE state IN ('running','finalizing')""" ).fetchall() for row in rows: (delegation_id, session_key, origin_ui, parent_id, dispatched_at, - pid, started, task_json, origin_session_id) = row + pid, started, task_json, origin_session_id, result_delivery) = row live = False if pid: live = _pid_exists(int(pid)) @@ -323,9 +500,13 @@ def recover_abandoned_delegations() -> int: # after a restart remain routable to api_server sessions. "origin_session_id": origin_session_id or "", "parent_session_id": parent_id, "goal": task.get("goal", ""), + "parent_turn_id": task.get("parent_turn_id", ""), "goals": task.get("goals"), "context": task.get("context"), "toolsets": task.get("toolsets"), "role": task.get("role"), "model": task.get("model"), "is_batch": bool(task.get("is_batch")), + "result_delivery": str( + result_delivery or task.get("result_delivery") or "after_turn" + ), "status": "unknown", "summary": None, "error": "Delegation owner exited before recording a terminal result; outcome unknown.", "dispatched_at": dispatched_at, "completed_at": now, @@ -354,18 +535,34 @@ def restore_undelivered_completions(target_queue) -> int: results seconds after boot (#64484). """ recover_abandoned_delegations() + restored = 0 with _DB_LOCK, _transaction() as conn: rows = conn.execute( """SELECT delegation_id, event_json FROM async_delegations WHERE state != 'running' AND delivery_state='pending' AND event_json IS NOT NULL ORDER BY completed_at, delegation_id""" ).fetchall() + child_rows = conn.execute( + """SELECT delegation_id, event_key, event_json + FROM async_delegation_events + WHERE delivery_state='pending' + ORDER BY created_at, delegation_id, event_key""" + ).fetchall() for _delegation_id, payload in rows: evt = json.loads(payload) if isinstance(evt, dict): evt["restored"] = True target_queue.put(evt) - return len(rows) + restored += 1 + for delegation_id, event_key, payload in child_rows: + evt = json.loads(payload) + if isinstance(evt, dict): + evt["restored"] = True + evt.setdefault("delegation_id", delegation_id) + evt.setdefault("delivery_event_key", event_key) + target_queue.put(evt) + restored += 1 + return restored def mark_completion_delivered(delegation_id: str) -> bool: @@ -400,6 +597,21 @@ def claim_completion_delivery(delegation_id: str, claim_id: str) -> bool: return cur.rowcount == 1 +def _claim_child_event(delegation_id: str, event_key: str, claim_id: str) -> bool: + now = time.time() + with _DB_LOCK, _transaction() as conn: + cur = conn.execute( + """UPDATE async_delegation_events + SET delivery_claim=?, delivery_claimed_at=?, + delivery_attempts=delivery_attempts+1, updated_at=? + WHERE delegation_id=? AND event_key=? + AND delivery_state='pending' + AND (delivery_claim IS NULL OR delivery_claimed_at < ?)""", + (claim_id, now, now, delegation_id, event_key, now - 300), + ) + return cur.rowcount == 1 + + def claim_event_delivery(evt: Dict[str, Any], consumer: str) -> Optional[str]: """Claim a durable delegation event; non-durable events need no token.""" if evt.get("type") != "async_delegation": @@ -408,7 +620,12 @@ def claim_event_delivery(evt: Dict[str, Any], consumer: str) -> Optional[str]: if not delegation_id: return "" claim_id = f"{consumer}:{__import__('os').getpid()}:{uuid.uuid4().hex}" - return claim_id if claim_completion_delivery(delegation_id, claim_id) else None + event_key = str(evt.get("delivery_event_key") or "") + if event_key: + claimed = _claim_child_event(delegation_id, event_key, claim_id) + else: + claimed = claim_completion_delivery(delegation_id, claim_id) + return claim_id if claimed else None def release_completion_delivery(delegation_id: str, claim_id: str) -> bool: @@ -484,14 +701,77 @@ def complete_completion_delivery(delegation_id: str, claim_id: str) -> bool: return cur.rowcount == 1 +def _complete_child_event( + delegation_id: str, event_key: str, claim_id: str, state: str = "delivered" +) -> bool: + now = time.time() + with _DB_LOCK, _transaction() as conn: + cur = conn.execute( + """UPDATE async_delegation_events + SET delivery_state=?, delivered_at=?, updated_at=?, + delivery_claim=NULL, delivery_claimed_at=NULL + WHERE delegation_id=? AND event_key=? + AND delivery_state='pending' AND delivery_claim=?""", + (state, now, now, delegation_id, event_key, claim_id), + ) + return cur.rowcount == 1 + + +def _release_child_event(delegation_id: str, event_key: str, claim_id: str) -> bool: + now = time.time() + with _DB_LOCK, _transaction() as conn: + capped = conn.execute( + """UPDATE async_delegation_events + SET delivery_state='dropped', delivery_claim=NULL, + delivery_claimed_at=NULL, updated_at=? + WHERE delegation_id=? AND event_key=? + AND delivery_state='pending' AND delivery_claim=? + AND delivery_attempts>=?""", + (now, delegation_id, event_key, claim_id, _MAX_DELIVERY_ATTEMPTS), + ) + if capped.rowcount == 1: + return True + cur = conn.execute( + """UPDATE async_delegation_events + SET delivery_claim=NULL, delivery_claimed_at=NULL, updated_at=? + WHERE delegation_id=? AND event_key=? + AND delivery_state='pending' AND delivery_claim=?""", + (now, delegation_id, event_key, claim_id), + ) + return cur.rowcount == 1 + + def complete_event_delivery(evt: Dict[str, Any], claim_id: str) -> None: - if claim_id and evt.get("type") == "async_delegation": - complete_completion_delivery(str(evt.get("delegation_id") or ""), claim_id) + if not claim_id or evt.get("type") != "async_delegation": + return + delegation_id = str(evt.get("delegation_id") or "") + event_key = str(evt.get("delivery_event_key") or "") + if event_key: + _complete_child_event(delegation_id, event_key, claim_id) + else: + complete_completion_delivery(delegation_id, claim_id) def release_event_delivery(evt: Dict[str, Any], claim_id: str) -> None: - if claim_id and evt.get("type") == "async_delegation": - release_completion_delivery(str(evt.get("delegation_id") or ""), claim_id) + if not claim_id or evt.get("type") != "async_delegation": + return + delegation_id = str(evt.get("delegation_id") or "") + event_key = str(evt.get("delivery_event_key") or "") + if event_key: + _release_child_event(delegation_id, event_key, claim_id) + else: + release_completion_delivery(delegation_id, claim_id) + + +def drop_event_delivery(evt: Dict[str, Any], claim_id: str) -> None: + if not claim_id or evt.get("type") != "async_delegation": + return + delegation_id = str(evt.get("delegation_id") or "") + event_key = str(evt.get("delivery_event_key") or "") + if event_key: + _complete_child_event(delegation_id, event_key, claim_id, state="dropped") + else: + drop_completion_delivery(delegation_id, claim_id) def get_durable_delegation(delegation_id: str) -> Optional[Dict[str, Any]]: @@ -636,6 +916,8 @@ def dispatch_async_delegation( interrupt_fn: Optional[Callable[[], None]] = None, max_async_children: int = _DEFAULT_MAX_ASYNC_CHILDREN, progress_fn: Optional[Callable[[], tuple]] = None, + result_delivery: Optional[str] = None, + parent_turn_id: Optional[str] = None, ) -> Dict[str, Any]: """Spawn ``runner`` on the daemon executor and return a handle immediately. @@ -701,6 +983,11 @@ def dispatch_async_delegation( "_progress_token": None, "_progress_ts": dispatched_at, "_interrupted_at": None, + # Delivery mode: 'inject' (synthetic notification at safe boundaries) + # or 'after_turn' (legacy completion_queue path). Default 'after_turn' + # for backward compatibility with old task_json entries. + "result_delivery": str(result_delivery or "after_turn").strip().lower(), + "parent_turn_id": str(parent_turn_id or ""), } # Capacity check and record insert under ONE lock hold β€” checking # active_count() separately would let two concurrent dispatches (e.g. @@ -839,6 +1126,7 @@ def _push_completion_event( "origin_ui_session_id": record.get("origin_ui_session_id", ""), "origin_session_id": record.get("origin_session_id", ""), "parent_session_id": record.get("parent_session_id"), + "parent_turn_id": record.get("parent_turn_id", ""), "goal": record.get("goal", ""), "context": record.get("context"), "toolsets": record.get("toolsets"), @@ -854,6 +1142,10 @@ def _push_completion_event( "dispatched_at": dispatched_at, "completed_at": completed_at, "exit_reason": result.get("exit_reason"), + # Delivery mode: 'inject' (synthetic at safe boundaries) or + # 'after_turn' (legacy completion_queue path). The drain side uses + # this to decide whether to inject now or wait for after-turn. + "result_delivery": record.get("result_delivery", "after_turn"), } # Structured stall metadata (#51690) β€” additive, present only on # stall-monitor finalizations. @@ -892,6 +1184,8 @@ def dispatch_async_delegation_batch( max_async_children: int = _DEFAULT_MAX_ASYNC_CHILDREN, delegation_id: Optional[str] = None, progress_fn: Optional[Callable[[], tuple]] = None, + result_delivery: Optional[str] = None, + parent_turn_id: Optional[str] = None, ) -> Dict[str, Any]: """Dispatch a WHOLE fan-out batch as ONE background unit. @@ -903,11 +1197,10 @@ def dispatch_async_delegation_batch( parallelism is bounded separately by ``max_concurrent_children``), so a single ``delegate_task`` fan-out never exhausts the async pool by itself. - When the batch finishes, a SINGLE completion event is pushed onto the - shared ``process_registry.completion_queue`` carrying the full per-task - ``results`` list, so the consolidated summaries re-enter the conversation - as one message once every child is done β€” the chat is never blocked while - they run. + ``after_turn`` publishes one consolidated completion after every child is + done. ``inject`` uses the same batch execution unit but each child publishes + an independently durable event as it becomes ready; finalization only + persists the aggregate result/status for observability and recovery. Returns ``{"status": "dispatched", "delegation_id": ...}`` on success or ``{"status": "rejected", "error": ...}`` when the async pool is at @@ -941,6 +1234,9 @@ def dispatch_async_delegation_batch( "_progress_token": None, "_progress_ts": dispatched_at, "_interrupted_at": None, + # Delivery mode for the batch; missing values preserve after_turn. + "result_delivery": str(result_delivery or "after_turn").strip().lower(), + "parent_turn_id": str(parent_turn_id or ""), } with _records_lock: running = sum( @@ -1044,6 +1340,7 @@ def _push_batch_completion_event( "origin_ui_session_id": event_record.get("origin_ui_session_id", ""), "origin_session_id": event_record.get("origin_session_id", ""), "parent_session_id": event_record.get("parent_session_id"), + "parent_turn_id": event_record.get("parent_turn_id", ""), "goal": event_record.get("goal", ""), "goals": event_record.get("goals"), "context": event_record.get("context"), @@ -1063,6 +1360,8 @@ def _push_batch_completion_event( "total_duration_seconds": combined.get("total_duration_seconds"), "dispatched_at": dispatched_at, "completed_at": completed_at, + # Delivery mode for the batch: 'inject' or 'after_turn'. + "result_delivery": event_record.get("result_delivery", "after_turn"), } # Structured stall metadata (#51690) β€” additive, present only on # stall-monitor finalizations. @@ -1075,6 +1374,26 @@ def _push_batch_completion_event( if _k in combined: evt[_k] = combined[_k] _persist_completion(evt, combined) + if str(event_record.get("result_delivery") or "after_turn").lower() == "inject": + # Child callbacks normally publish immediately. Re-publish here as an + # idempotent safety net for callback failures, then acknowledge the + # aggregate parent row so restart recovery cannot replay a duplicate + # combined event. + delegation_id = str(event_record.get("delegation_id") or "") + for child in combined.get("results") or []: + try: + publish_batch_child_completion( + delegation_id, + int(child.get("task_index", 0)), + child, + ) + except Exception: + logger.exception( + "Failed to persist inject child event for %s", delegation_id + ) + _publish_batch_terminal_event(event_record, combined, status) + mark_completion_delivered(delegation_id) + return try: process_registry.completion_queue.put(evt) except Exception as exc: # pragma: no cover diff --git a/tools/delegate_tool.py b/tools/delegate_tool.py index 30151b429c42..2caea4961239 100644 --- a/tools/delegate_tool.py +++ b/tools/delegate_tool.py @@ -2782,6 +2782,7 @@ def delegate_task( max_iterations: Optional[int] = None, role: Optional[str] = None, background: Optional[bool] = None, + result_delivery: Optional[str] = None, parent_agent=None, ) -> str: """ @@ -2813,15 +2814,23 @@ def delegate_task( # Normalise the top-level role once; per-task overrides re-normalise. top_role = _normalize_role(role) - # Background (async) delegation now applies to BOTH single tasks and - # batches. A batch is dispatched as ONE async unit: the whole fan-out runs - # on the daemon executor, joins on every child (see _execute_and_aggregate - # / dispatch_async_delegation_batch), and pushes a SINGLE completion event - # carrying the consolidated per-task results. It re-enters the conversation - # as one message once ALL children finish β€” the chat is not blocked while - # they run. + # Background delegation applies to both single tasks and batches. A top-level + # call is one async execution/stall unit. result_delivery determines whether + # ready children surface independently at safe boundaries or the completed + # batch follows the legacy consolidated synthetic-turn path. background = is_truthy_value(background, default=False) if background is not None else False + # result_delivery controls how child results reach the parent: + # 'inject' β€” synthetic user notification at safe conversation-loop + # boundaries (after tool-results, before next LLM call). + # 'after_turn' β€” legacy completion_queue path; surfaces as a new turn + # only after the current agent turn fully completes. + # Default is 'after_turn' for backward compatibility: old task_json entries + # that lack this field are treated as after_turn. + _delivery = str(result_delivery or "").strip().lower() + if _delivery not in {"inject", "after_turn"}: + _delivery = "after_turn" + # Depth limit β€” configurable via delegation.max_spawn_depth, # default 2 for parity with the original MAX_DEPTH constant. depth = getattr(parent_agent, "_delegate_depth", 0) @@ -2917,6 +2926,12 @@ def delegate_task( live_deleg_id, live_writers, live_paths = create_live_transcripts( task_list, context ) + if not live_deleg_id: + # Child-level durable delivery needs a stable id even when the optional + # live-transcript side channel could not be created. + from tools.async_delegation import _new_delegation_id + + live_deleg_id = _new_delegation_id() # Capture the ORIGINATING session's wake target BEFORE any child agent is # constructed: _build_child_agent() -> AIAgent() -> agent_init calls @@ -2974,6 +2989,27 @@ def delegate_task( child._live_transcript_path = str(_writer.path) children.append((i, t, child)) + def _publish_ready_result(entry: Dict[str, Any]) -> None: + """Publish one inject child without waiting for its batch siblings.""" + if not background or _delivery != "inject": + return + task_index = int(entry.get("task_index", 0)) + payload = dict(entry) + if 0 <= task_index < len(live_paths): + payload.setdefault("live_transcript", live_paths[task_index]) + try: + from tools.async_delegation import publish_batch_child_completion + + publish_batch_child_completion(live_deleg_id, task_index, payload) + except Exception: + # _push_batch_completion_event republishes all children + # idempotently as a durable safety net. + logger.exception( + "Failed to publish ready inject child %s/%s", + live_deleg_id, + task_index, + ) + def _execute_and_aggregate(*, honor_parent_interrupt: bool = True) -> dict: """Run all built children (1 or N), join on them, aggregate results, fire subagent_stop hooks + cost rollup, and return the combined result @@ -2989,6 +3025,7 @@ def _execute_and_aggregate(*, honor_parent_interrupt: bool = True) -> dict: _i, _t, child = children[0] result = _run_single_child(_i, _t["goal"], child, parent_agent) results.append(result) + _publish_ready_result(result) else: # Batch -- run in parallel with per-task progress lines completed_count = 0 @@ -3060,6 +3097,7 @@ def _execute_and_aggregate(*, honor_parent_interrupt: bool = True) -> dict: ), } results.append(entry) + _publish_ready_result(entry) completed_count += 1 break @@ -3085,6 +3123,7 @@ def _execute_and_aggregate(*, honor_parent_interrupt: bool = True) -> dict: ), } results.append(entry) + _publish_ready_result(entry) completed_count += 1 # Print per-task completion line above the spinner @@ -3154,12 +3193,9 @@ def _execute_and_aggregate(*, honor_parent_interrupt: bool = True) -> dict: return combined # ----- Background dispatch: run the WHOLE batch as one async unit ----- - # When background is true, the entire fan-out runs on the daemon executor - # via a single async delegation. _execute_and_aggregate() joins on every - # child and produces ONE consolidated results block, which re-enters the - # conversation as a single message when ALL children finish. The chat is - # not blocked in the meantime. This is the contract: dispatch N subagents, - # keep chatting, get the combined summaries back together at the end. + # _execute_and_aggregate owns child execution and still returns one ordered + # aggregate. In inject mode its completion callback also publishes each + # child immediately; in after_turn mode only the aggregate is delivered. if background: from tools.async_delegation import dispatch_async_delegation_batch from tools.approval import get_current_session_key @@ -3250,6 +3286,7 @@ def _execute_and_aggregate(*, honor_parent_interrupt: bool = True) -> dict: if _agent_session_id: _session_key = _agent_session_id _parent_session_id = getattr(parent_agent, "session_id", None) + _parent_turn_id = str(getattr(parent_agent, "_active_turn_id", "") or "") _child_agents = [c for (_, _, c) in children] # Detach every child from the parent's interrupt-propagation list β€” the @@ -3334,25 +3371,43 @@ def _batch_progress(): # returned delegation_id matches cache/delegation/live//. delegation_id=live_deleg_id, progress_fn=_batch_progress, + # Delivery mode: 'inject' (synthetic at safe boundaries) or + # 'after_turn' (legacy completion_queue path). Default 'after_turn' + # preserves backward compatibility with old task_json entries. + result_delivery=_delivery, + parent_turn_id=_parent_turn_id, ) if dispatch.get("status") == "dispatched": n = len(_goals) - note = ( - "Subagent is running in the background. You and the user can " - "keep working; its full result re-enters the conversation as a " - "new message when it finishes. Do not wait or poll β€” just " - "continue." - if n == 1 else - f"{n} subagents are running in parallel in the background. You " - f"and the user can keep working; they wait on each other and " - f"their consolidated results re-enter the conversation as a " - f"single message once ALL of them finish. Do not wait or poll " - f"β€” just continue." - ) + if _delivery == "inject": + note = ( + "Subagent is running asynchronously. Keep working; each ready " + "result will be injected at the next safe model boundary in " + "this turn. A result that finishes after this turn becomes a " + "separate synthetic turn. Never wait or poll." + if n == 1 else + f"{n} subagents are running asynchronously. Keep working; " + "each child result is injected independently at the next " + "safe model boundary without waiting for slower siblings. " + "Late results become separate synthetic turns. Never wait or poll." + ) + else: + note = ( + "Subagent is running in the background. You and the user can " + "keep working; its full result re-enters the conversation as a " + "new message when it finishes. Do not wait or poll β€” just " + "continue." + if n == 1 else + f"{n} subagents are running in parallel in the background. You " + f"and the user can keep working; their consolidated results " + f"re-enter as a single message once ALL finish. Do not wait " + f"or poll β€” just continue." + ) payload = { "status": "dispatched", "mode": "background", + "result_delivery": _delivery, "count": n, "delegation_id": dispatch["delegation_id"], "goals": _goals, @@ -3703,11 +3758,14 @@ def _build_top_level_description() -> str: f"items concurrently for this user (configured via " f"delegation.max_concurrent_children in config.yaml). {nesting_clause}\n\n" "BOTH MODES RUN IN THE BACKGROUND. delegate_task returns immediately β€” " - "you and the user keep working, and the completed result re-enters " - "the conversation as a new message. A " - "batch returns one handle, runs N subagents concurrently, and delivers " - "one consolidated result after ALL of them finish. Do NOT wait or poll; " - "just continue with other work after dispatching.\n\n" + "you and the user keep working. Choose result_delivery='inject' for an " + "auditor, reviewer, or dependent task whose result can change THIS turn: " + "each ready child is appended only after complete tool results and before " + "the next model request, including one final reconciliation boundary. " + "Choose result_delivery='after_turn' (the default) for independent work: " + "single results and consolidated batches arrive as separate synthetic " + "turns after the foreground turn. Neither mode waits for running children. " + "Do NOT wait or poll; continue after dispatching.\n\n" "LIVE TRANSCRIPTS: the dispatch response includes 'live_transcripts' β€” " "one append-only human-readable log file per task (under " "cache/delegation/live//). Each child streams its " @@ -3906,6 +3964,22 @@ def _build_dynamic_schema_overrides() -> dict: "backward compatibility." ), }, + "result_delivery": { + "type": "string", + "enum": ["inject", "after_turn"], + "default": "after_turn", + "description": ( + "How child results are delivered back to the parent context. " + "'inject': use for auditors, reviewers, and dependent work " + "whose result must be read before the parent continues or " + "finalizes; ready results are appended only at safe boundaries " + "after complete tool-result sequences and before the next model " + "request. Batch children inject independently as they finish. " + "'after_turn' (default): use for independent background work; " + "the result arrives in a separate synthetic turn after the " + "foreground turn. Running children are never waited on." + ), + }, }, "required": [], }, diff --git a/tools/process_registry.py b/tools/process_registry.py index 7daf74b2e336..411f26c9be12 100644 --- a/tools/process_registry.py +++ b/tools/process_registry.py @@ -2126,24 +2126,35 @@ def _format_async_delegation(evt: dict) -> str: dispatched_at = evt.get("dispatched_at") completed_at = evt.get("completed_at") or _time.time() - # ----- Batch (fan-out) completion: consolidated multi-task block ----- - # A whole delegate_task fan-out dispatched as one background unit finishes - # together and carries a per-task `results` list. Render every subagent's - # summary in one block so the model gets the consolidated outcome at once. + # ----- Batch result: aggregate after_turn or per-child inject block ----- + # Aggregate events carry every result; inject child events carry one result + # plus its original batch index. Both use the same stable formatter. batch_results = evt.get("results") if evt.get("is_batch") or isinstance(batch_results, list): results = batch_results or [] goals = evt.get("goals") or [] - n = len(results) if results else len(goals) + child_event = str(evt.get("delivery_event_key") or "").startswith("task:") + n = int(evt.get("batch_size") or 0) if child_event else 0 + if n <= 0: + n = len(results) if results else len(goals) total_dur = evt.get("total_duration_seconds", duration) - lines = [ - f"[ASYNC DELEGATION BATCH COMPLETE β€” {deleg_id}]", - f"A background fan-out of {n} subagent(s) you dispatched earlier " - "has finished. All ran in parallel and waited on each other; their " - "consolidated results are below. You may have moved on since " - "dispatching β€” act on these or re-dispatch if things have changed.", - "", - ] + if child_event: + child_idx = int(evt.get("task_index") or 0) + lines = [ + f"[ASYNC DELEGATION RESULT READY β€” {deleg_id} β€” TASK {child_idx + 1}/{n}]", + "A background subagent result is ready. It was requested for " + "same-turn reconciliation; use it before continuing the current work.", + "", + ] + else: + lines = [ + f"[ASYNC DELEGATION BATCH COMPLETE β€” {deleg_id}]", + f"A background fan-out of {n} subagent(s) you dispatched earlier " + "has finished. All ran in parallel and waited on each other; their " + "consolidated results are below. You may have moved on since " + "dispatching β€” act on these or re-dispatch if things have changed.", + "", + ] if isinstance(dispatched_at, (int, float)): ts = _time.strftime("%Y-%m-%d %H:%M:%S", _time.localtime(dispatched_at)) age = f" ({_format_age(completed_at - dispatched_at)} ago)" diff --git a/website/docs/guides/delegation-patterns.md b/website/docs/guides/delegation-patterns.md index 9f16bb34999f..af89abb156a7 100644 --- a/website/docs/guides/delegation-patterns.md +++ b/website/docs/guides/delegation-patterns.md @@ -84,10 +84,18 @@ delegate_task( Auth files: src/auth/login.py, src/auth/jwt.py, src/auth/middleware.py Test command: pytest tests/auth/ -v Focus on: SQL injection, JWT validation, password hashing, session management. - Fix issues found and verify tests pass.""" + Fix issues found and verify tests pass.""", + result_delivery="inject", ) ``` +`inject` is appropriate here because the review can change the parent's current +implementation or final answer. Hermes does not wait for the reviewer: if the +review finishes during this foreground turn, the parent reconciles it at the +next safe model boundary; otherwise the result arrives later as its own turn. +Use the default `after_turn` for independent work whose result does not need to +change the current turn. + :::warning The Context Problem Subagents know **absolutely nothing** about your conversation. They start completely fresh. If you delegate "fix the bug we were discussing," the subagent has no idea what bug you mean. Always pass file paths, error messages, project structure, and constraints explicitly. ::: diff --git a/website/docs/user-guide/features/delegation.md b/website/docs/user-guide/features/delegation.md index 14fbad48058e..4f11736fa54b 100644 --- a/website/docs/user-guide/features/delegation.md +++ b/website/docs/user-guide/features/delegation.md @@ -8,7 +8,7 @@ description: "Spawn isolated child agents for parallel workstreams with delegate The `delegate_task` tool spawns child AIAgent instances with isolated context, inherited tool access, and their own terminal sessions. Each child gets a fresh conversation and works independently β€” only its final summary enters the parent's context. -Top-level model calls run in the background automatically. Hermes returns a handle immediately so the conversation can continue, then posts the result back as a new message. An orchestrator subagent waits for its own workers so it can synthesize their results before returning. +Top-level model calls run in the background automatically. Hermes returns a handle immediately so the conversation can continue. The model chooses how the result comes back with `result_delivery`: the backward-compatible default, `after_turn`, posts a separate synthetic turn; `inject` lets an auditor or dependency re-enter the still-running parent turn at its next safe model boundary. An orchestrator subagent still waits for its own workers so it can synthesize their results before returning. ## Single Task @@ -31,6 +31,37 @@ delegate_task(tasks=[ ]) ``` +## Result Delivery + +`delegate_task` is always asynchronous at the top level and never waits for a +running child. `result_delivery` controls only when an already-completed result +is shown to the parent model: + +- **`after_turn` (default):** preserves the existing behavior. A single result, + or one consolidated batch result, is delivered as a separate synthetic turn + after the foreground turn ends. +- **`inject`:** intended for auditors, reviewers, and dependent work that can + change what the parent should do now. Each ready child is appended to the + conversation at the next safe boundary, after all tool results from the + current assistant message and before the next model request. Batch children + do not wait for slower siblings. If a child finishes after its originating + turn has ended, it falls back to a separate synthetic turn instead of being + spliced into an unrelated later turn. + +```python +delegate_task( + goal="Audit the patch for correctness and race conditions", + context="Project: /home/user/project; review the current uncommitted diff", + result_delivery="inject", +) +``` + +Injection is append-only: Hermes never rewrites earlier history or interrupts a +tool-call/result sequence. Multiple results already ready at one boundary are +coalesced into one synthetic user message, then the parent model receives a +reconciliation request. If no result is ready, the foreground turn finalizes +normally; Hermes does not poll or extend it just to wait for a subagent. + ## How Subagent Context Works :::warning Critical: Subagents Know Nothing @@ -115,7 +146,7 @@ delegate_task( ## Batch Mode Details -When a top-level agent provides a `tasks` array, Hermes returns one background handle, runs the subagents in parallel, and posts one consolidated result after every child finishes. An orchestrator subagent waits for its batch in the current turn so it can synthesize the results. +When a top-level agent provides a `tasks` array, Hermes returns one background handle and runs the subagents in parallel. With the default `after_turn` delivery it posts one consolidated result after every child finishes. With `inject`, each child summary can re-enter independently as soon as it is ready. An orchestrator subagent waits for its batch in the current turn so it can synthesize the results. - **Maximum concurrency:** 3 tasks by default (configurable via `delegation.max_concurrent_children` or the `DELEGATION_MAX_CONCURRENT_CHILDREN` env var; floor of 1, no hard ceiling). Batches larger than the limit return a tool error rather than being silently truncated. - **Thread pool:** Uses `ThreadPoolExecutor` with the configured concurrency limit as max workers @@ -128,11 +159,13 @@ Synchronous single-task delegation from an orchestrator runs directly without th ### Durable background completions When a background delegation finishes, Hermes stores its completion event in -the active profile's `state.db` before publishing it to the normal fresh-turn -queue. If Hermes restarts after completion but before delivery, the pending -event is restored and routed through the same ownership checks. Competing -consumers use a durable claim, so only the consumer that successfully accepts -the synthetic turn acknowledges delivery; failed attempts release the claim for +the active profile's `state.db` before publishing it to the shared completion +queue. `inject` batches use one execution record plus independently claimable +child-delivery records; `after_turn` keeps one aggregate delivery record. If +Hermes restarts after completion but before delivery, pending events are +restored and routed through the same ownership checks. Competing consumers use +a durable claim, so only the consumer that successfully appends or accepts the +synthetic turn acknowledges each event; failed attempts release the claim for retry. This does not resume child execution after a crash. A delegation whose owner From ceed31836bd949968e084e5c0f39ca90f4d4afe6 Mon Sep 17 00:00:00 2001 From: Xipong <217837358+Xipong@users.noreply.github.com> Date: Thu, 30 Jul 2026 01:04:20 +0300 Subject: [PATCH 2/7] fix(delegation): harden result delivery recovery --- agent/delegation_inject.py | 50 +++++++++++++--- tests/agent/test_delegation_inject.py | 58 +++++++++++++++++++ tools/async_delegation.py | 49 ++++++++++------ tools/delegate_tool.py | 11 ++-- .../docs/user-guide/features/delegation.md | 7 ++- 5 files changed, 143 insertions(+), 32 deletions(-) diff --git a/agent/delegation_inject.py b/agent/delegation_inject.py index ff1402b30e25..7ee00947ca5b 100644 --- a/agent/delegation_inject.py +++ b/agent/delegation_inject.py @@ -15,14 +15,43 @@ logger = logging.getLogger(__name__) -def _grant_reconciliation_grace_if_needed(agent: Any) -> None: - """Allow one request only when the normal loop budget is exhausted.""" +_GRACE_TURN_ATTR = "_delegation_reconciliation_grace_turn_id" - api_calls = int(getattr(agent, "_api_call_count", 0) or 0) - max_iterations = int(getattr(agent, "max_iterations", 0) or 0) + +def _normal_budget_available(agent: Any) -> bool: + """Mirror the conversation-loop's normal iteration-budget predicate.""" + + max_iterations = getattr(agent, "max_iterations", None) budget = getattr(agent, "iteration_budget", None) + # Lightweight helper users/tests do not necessarily expose loop-budget + # state. In production both attributes exist; absent state must not make a + # non-blocking queue drain manufacture a grace-call contract of its own. + if max_iterations is None or budget is None: + return True + api_calls = int(getattr(agent, "_api_call_count", 0) or 0) remaining = int(getattr(budget, "remaining", 0) or 0) - if (max_iterations and api_calls >= max_iterations) or remaining <= 0: + return api_calls < int(max_iterations or 0) and remaining > 0 + + +def _has_reconciliation_capacity(agent: Any, turn_id: str) -> bool: + """Return whether one more model request can consume an inject event.""" + + if _normal_budget_available(agent): + return True + # A generic budget grace already granted by the loop can carry this inject; + # record it as this turn's sole reconciliation boundary after acceptance. + if bool(getattr(agent, "_budget_grace_call", False)): + return True + return str(getattr(agent, _GRACE_TURN_ATTR, "") or "") != str(turn_id) + + +def _grant_reconciliation_grace_if_needed(agent: Any, turn_id: str) -> None: + """Reserve at most one exhausted-budget reconciliation call per turn.""" + + if _normal_budget_available(agent): + return + setattr(agent, _GRACE_TURN_ATTR, str(turn_id)) + if not bool(getattr(agent, "_budget_grace_call", False)): agent._budget_grace_call = True @@ -48,7 +77,7 @@ def reconcile_provisional_final( # One reconciliation request must remain possible if the provisional final # consumed the nominal iteration budget. This grants no waiting and does not # bypass a budget that still has room. - _grant_reconciliation_grace_if_needed(agent) + _grant_reconciliation_grace_if_needed(agent, turn_id) return True @@ -71,6 +100,13 @@ def drain_ready_injects(agent: Any, messages: list[dict[str, Any]], turn_id: str # an assistant response establishes the next append-only boundary. if messages and messages[-1].get("role") == "user": return 0 + # Once this turn has consumed its sole exhausted-budget reconciliation + # request, a later child must remain pending. The gateway/idle watcher will + # deliver it through the normal late-result turn after the parent exits; + # appending it here would mark it delivered even though no model call could + # read it. + if not _has_reconciliation_capacity(agent, turn_id): + return 0 from tools.async_delegation import ( claim_event_delivery, @@ -151,5 +187,5 @@ def drain_ready_injects(agent: Any, messages: list[dict[str, Any]], turn_id: str for event, claim_id, _text in accepted: complete_event_delivery(event, claim_id) - _grant_reconciliation_grace_if_needed(agent) + _grant_reconciliation_grace_if_needed(agent, turn_id) return len(accepted) diff --git a/tests/agent/test_delegation_inject.py b/tests/agent/test_delegation_inject.py index fb6bd8e901e0..ccb3687ffa2d 100644 --- a/tests/agent/test_delegation_inject.py +++ b/tests/agent/test_delegation_inject.py @@ -218,6 +218,40 @@ def test_retry_tail_user_defers_new_inject_without_rewriting_history(): assert [event["delivery_event_key"] for event in _queue_contents()] == ["task:1"] +def test_exhausted_budget_grants_only_one_reconciliation_per_turn(): + delegation_id = _record(goals=("first", "second")) + agent = SimpleNamespace( + _active_turn_id="turn-current", + _budget_grace_call=False, + _api_call_count=1, + max_iterations=1, + iteration_budget=SimpleNamespace(remaining=0), + ) + messages = [{"role": "assistant", "content": "working"}] + + assert ad.publish_batch_child_completion( + delegation_id, 0, _child(0, "first result") + ) + assert drain_ready_injects(agent, messages, "turn-current") == 1 + assert agent._budget_grace_call is True + + # Simulate the conversation loop consuming the one reconciliation request + # and producing an assistant response. A child that finishes afterwards + # must stay pending for the late/after-turn delivery path instead of being + # acknowledged without any model request left to read it. + agent._budget_grace_call = False + messages.append({"role": "assistant", "content": "reconciled first result"}) + assert ad.publish_batch_child_completion( + delegation_id, 1, _child(1, "second result") + ) + + snapshot = deepcopy(messages) + assert drain_ready_injects(agent, messages, "turn-current") == 0 + assert messages == snapshot + assert [event["delivery_event_key"] for event in _queue_contents()] == ["task:1"] + assert _event_state(delegation_id, "task:1") == ("pending", 0) + + def test_inject_batch_publishes_first_child_without_waiting_for_sibling(): gate = threading.Event() ready = threading.Event() @@ -276,6 +310,15 @@ def runner(): time.sleep(0.01) assert ad.get_durable_delegation(delegation_id)["delivery_state"] == "delivered" + restored_queue = __import__("queue").Queue() + assert ad.restore_undelivered_completions(restored_queue) == 2 + restored = [restored_queue.get_nowait(), restored_queue.get_nowait()] + assert {event.get("delivery_event_key") for event in restored} == { + "task:0", + "task:1", + } + assert all(event.get("delivery_event_key") for event in restored) + def test_after_turn_remains_default_and_emits_only_combined_batch_event(): gate = threading.Event() @@ -358,3 +401,18 @@ def fake_delegate_task(**kwargs): assert result == "ok" assert captured["background"] is True assert captured["result_delivery"] == "inject" + + # The registry fallback is a distinct model-facing dispatch path. It must + # preserve the same delivery choice if the run_agent intercept is bypassed. + captured.clear() + from tools.registry import registry + + entry = registry.get_entry("delegate_task") + assert entry is not None + result = entry.handler( + {"goal": "audit", "result_delivery": "inject"}, + parent_agent=SimpleNamespace(_delegate_depth=0), + ) + assert result == "ok" + assert captured["background"] is True + assert captured["result_delivery"] == "inject" diff --git a/tools/async_delegation.py b/tools/async_delegation.py index 5572f5151d2a..e827ab467c5e 100644 --- a/tools/async_delegation.py +++ b/tools/async_delegation.py @@ -316,15 +316,30 @@ def _prune_durable_records() -> None: ) -def _persist_completion(event: Dict[str, Any], result: Dict[str, Any]) -> None: +def _persist_completion( + event: Dict[str, Any], + result: Dict[str, Any], + *, + delivery_state: str = "pending", +) -> None: now = time.time() + state = "delivered" if delivery_state == "delivered" else "pending" + delivered_at = now if state == "delivered" else None with _DB_LOCK, _transaction() as conn: conn.execute( """UPDATE async_delegations SET state=?, completed_at=?, updated_at=?, - event_json=?, result_json=?, delivery_state='pending' + event_json=?, result_json=?, delivery_state=?, delivered_at=? WHERE delegation_id=?""", - (event.get("status", "completed"), event.get("completed_at", now), now, - json.dumps(event), json.dumps(result), event["delegation_id"]), + ( + event.get("status", "completed"), + event.get("completed_at", now), + now, + json.dumps(event), + json.dumps(result), + state, + delivered_at, + event["delegation_id"], + ), ) @@ -1373,27 +1388,23 @@ def _push_batch_completion_event( ): if _k in combined: evt[_k] = combined[_k] - _persist_completion(evt, combined) if str(event_record.get("result_delivery") or "after_turn").lower() == "inject": # Child callbacks normally publish immediately. Re-publish here as an - # idempotent safety net for callback failures, then acknowledge the - # aggregate parent row so restart recovery cannot replay a duplicate - # combined event. + # idempotent safety net for callback failures. Persist every child event + # before atomically acknowledging the aggregate parent row: otherwise a + # crash between a pending aggregate write and a separate acknowledgement + # could restore both the aggregate and its child events after restart. delegation_id = str(event_record.get("delegation_id") or "") for child in combined.get("results") or []: - try: - publish_batch_child_completion( - delegation_id, - int(child.get("task_index", 0)), - child, - ) - except Exception: - logger.exception( - "Failed to persist inject child event for %s", delegation_id - ) + publish_batch_child_completion( + delegation_id, + int(child.get("task_index", 0)), + child, + ) _publish_batch_terminal_event(event_record, combined, status) - mark_completion_delivered(delegation_id) + _persist_completion(evt, combined, delivery_state="delivered") return + _persist_completion(evt, combined) try: process_registry.completion_queue.put(evt) except Exception as exc: # pragma: no cover diff --git a/tools/delegate_tool.py b/tools/delegate_tool.py index 2caea4961239..be1dd3081260 100644 --- a/tools/delegate_tool.py +++ b/tools/delegate_tool.py @@ -3971,10 +3971,12 @@ def _build_dynamic_schema_overrides() -> dict: "description": ( "How child results are delivered back to the parent context. " "'inject': use for auditors, reviewers, and dependent work " - "whose result must be read before the parent continues or " - "finalizes; ready results are appended only at safe boundaries " - "after complete tool-result sequences and before the next model " - "request. Batch children inject independently as they finish. " + "whose result can affect the current turn; a result that is " + "ready in time is appended only at a safe boundary after complete " + "tool-result sequences and before the next model request. Batch " + "children inject independently as they finish; results that miss " + "the bounded reconciliation window become separate late-result " + "turns. " "'after_turn' (default): use for independent background work; " "the result arrives in a separate synthetic turn after the " "foreground turn. Running children are never waited on." @@ -4040,6 +4042,7 @@ def _strip_model_hidden_task_fields(tasks: Any) -> Any: max_iterations=args.get("max_iterations"), role=args.get("role"), background=_model_background_value(args, kw.get("parent_agent")), + result_delivery=args.get("result_delivery"), parent_agent=kw.get("parent_agent"), ), check_fn=check_delegate_requirements, diff --git a/website/docs/user-guide/features/delegation.md b/website/docs/user-guide/features/delegation.md index 4f11736fa54b..80dc8152bf65 100644 --- a/website/docs/user-guide/features/delegation.md +++ b/website/docs/user-guide/features/delegation.md @@ -59,8 +59,11 @@ delegate_task( Injection is append-only: Hermes never rewrites earlier history or interrupts a tool-call/result sequence. Multiple results already ready at one boundary are coalesced into one synthetic user message, then the parent model receives a -reconciliation request. If no result is ready, the foreground turn finalizes -normally; Hermes does not poll or extend it just to wait for a subagent. +reconciliation request. If the normal iteration budget is exhausted, at most +one extra reconciliation request is granted for the originating turn; children +that finish after that bounded window stay pending and arrive as separate late- +result turns. If no result is ready, the foreground turn finalizes normally; +Hermes does not poll or extend it just to wait for a subagent. ## How Subagent Context Works From b5416248af8a61b6f616610f1059115c15c893ca Mon Sep 17 00:00:00 2001 From: Xipong <217837358+Xipong@users.noreply.github.com> Date: Thu, 30 Jul 2026 17:09:49 +0300 Subject: [PATCH 3/7] fix(delegation): harden inject crash recovery --- agent/conversation_loop.py | 35 +++ agent/delegation_inject.py | 167 +++++++++++--- tests/agent/test_delegation_inject.py | 203 ++++++++++++++++- tools/async_delegation.py | 307 +++++++++++++++++++------- 4 files changed, 603 insertions(+), 109 deletions(-) diff --git a/agent/conversation_loop.py b/agent/conversation_loop.py index eee7ecd2d4d9..3935bbb76bb0 100644 --- a/agent/conversation_loop.py +++ b/agent/conversation_loop.py @@ -1285,6 +1285,12 @@ def run_conversation( # Check for interrupt request (e.g., user sent new message) if agent._interrupt_requested: + try: + from agent.delegation_inject import release_pending_injects + + release_pending_injects(agent, messages, turn_id=turn_id) + except Exception: + logger.debug("Failed to release interrupted inject claims", exc_info=True) interrupted = True _turn_exit_reason = "interrupted_by_user" if not agent.quiet_mode: @@ -5364,6 +5370,15 @@ def _perform_api_call(next_api_kwargs): agent._vprint(f"{agent.log_prefix}⚑ Interrupt detected during retry wait, aborting.", force=True) _interrupt_text = f"Operation interrupted: retrying API call after error (retry {retry_count}/{max_retries})." close_interrupted_tool_sequence(messages, _interrupt_text) + try: + from agent.delegation_inject import release_pending_injects + + release_pending_injects(agent, messages, turn_id=turn_id) + except Exception: + logger.debug( + "Failed to release backoff-interrupted inject claims", + exc_info=True, + ) agent._persist_session(messages, conversation_history) agent.clear_interrupt() return { @@ -5467,6 +5482,15 @@ def _perform_api_call(next_api_kwargs): normalized = _transport.normalize_response(response, **_normalize_kwargs) assistant_message = normalized finish_reason = normalized.finish_reason + # A normalized provider response is the acceptance boundary for any + # synthetic delegation result in this request. Until here its + # durable row remains pending+claimed so a crash can recover it. + try: + from agent.delegation_inject import acknowledge_pending_injects + + acknowledge_pending_injects(agent, turn_id=turn_id) + except Exception: + logger.debug("Failed to acknowledge consumed inject claims", exc_info=True) # Normalize content to string β€” some OpenAI-compatible servers # (llama-server, etc.) return content as a dict or list instead @@ -7057,6 +7081,17 @@ def _perform_api_call(next_api_kwargs): messages.append({"role": "assistant", "content": final_response}) break + # Any claim left here was never consumed by a normalized provider response. + # Drop its RAM-only synthetic message and return the durable event to the + # queue. If a persistence boundary already saved it, that transcript row + # is the durable handoff and the helper acknowledges rather than requeues. + try: + from agent.delegation_inject import release_pending_injects + + release_pending_injects(agent, messages, turn_id=turn_id) + except Exception: + logger.debug("Failed to settle unconsumed inject claims", exc_info=True) + # Post-loop turn finalization extracted to agent/turn_finalizer.finalize_turn # (god-file decomposition Phase 1 step 4). Behavior-neutral: the assembled # result dict is returned exactly as before. diff --git a/agent/delegation_inject.py b/agent/delegation_inject.py index 7ee00947ca5b..334021a83bf7 100644 --- a/agent/delegation_inject.py +++ b/agent/delegation_inject.py @@ -16,6 +16,93 @@ _GRACE_TURN_ATTR = "_delegation_reconciliation_grace_turn_id" +_PENDING_CLAIMS_ATTR = "_pending_delegation_inject_claims" + + +def _event_identity(event: dict[str, Any]) -> str: + return ( + f"{event.get('delegation_id') or ''}:" + f"{event.get('delivery_event_key') or 'aggregate'}" + ) + + +def _message_event_ids(message: dict[str, Any]) -> set[str]: + metadata = message.get("display_metadata") + if isinstance(metadata, dict): + values = metadata.get("delegation_event_ids") or [] + else: + values = message.get("_delegation_event_ids") or [] + return {str(value) for value in values if value} + + +def _durable_event_is_in_history( + messages: list[dict[str, Any]], event_id: str +) -> bool: + return any( + message.get("_db_persisted") is True + and event_id in _message_event_ids(message) + for message in messages + if isinstance(message, dict) + ) + + +def acknowledge_pending_injects(agent: Any, *, turn_id: str | None = None) -> int: + """Acknowledge inject claims after a provider consumed their message.""" + + from tools.async_delegation import complete_event_delivery + + pending = list(getattr(agent, _PENDING_CLAIMS_ATTR, []) or []) + keep: list[dict[str, Any]] = [] + acknowledged = 0 + for entry in pending: + if turn_id is not None and str(entry.get("turn_id") or "") != str(turn_id): + keep.append(entry) + continue + complete_event_delivery(entry["event"], entry["claim_id"]) + acknowledged += 1 + setattr(agent, _PENDING_CLAIMS_ATTR, keep) + return acknowledged + + +def release_pending_injects( + agent: Any, + messages: list[dict[str, Any]], + *, + turn_id: str | None = None, +) -> int: + """Roll back unconsumed RAM injects, preserving already-durable copies.""" + + from tools.async_delegation import ( + complete_event_delivery, + release_event_delivery, + ) + from tools.process_registry import process_registry + + pending = list(getattr(agent, _PENDING_CLAIMS_ATTR, []) or []) + keep: list[dict[str, Any]] = [] + removable_message_ids: set[int] = set() + released = 0 + for entry in pending: + if turn_id is not None and str(entry.get("turn_id") or "") != str(turn_id): + keep.append(entry) + continue + event = entry["event"] + event_id = str(entry["event_id"]) + if _durable_event_is_in_history(messages, event_id): + complete_event_delivery(event, entry["claim_id"]) + else: + release_event_delivery(event, entry["claim_id"]) + process_registry.completion_queue.put(event) + removable_message_ids.add(id(entry["message"])) + released += 1 + + if removable_message_ids: + messages[:] = [ + message for message in messages if id(message) not in removable_message_ids + ] + agent._session_messages = messages + setattr(agent, _PENDING_CLAIMS_ATTR, keep) + return released def _normal_budget_available(agent: Any) -> bool: @@ -93,6 +180,16 @@ def drain_ready_injects(agent: Any, messages: list[dict[str, Any]], turn_id: str if not turn_id: return 0 + # A cached agent may enter a new turn after an early-return path that never + # reached the common finalizer. Settle only claims from older turns; claims + # from this turn must survive compression/API retries without duplication. + stale_turn_ids = { + str(entry.get("turn_id") or "") + for entry in (getattr(agent, _PENDING_CLAIMS_ATTR, []) or []) + if str(entry.get("turn_id") or "") != str(turn_id) + } + for stale_turn_id in stale_turn_ids: + release_pending_injects(agent, messages, turn_id=stale_turn_id) # A previous inject may already be the tail while its reconciliation API # request is being retried after a transport failure. Appending another # synthetic user message here would create userβ†’user history and force the @@ -121,7 +218,7 @@ def drain_ready_injects(agent: Any, messages: list[dict[str, Any]], turn_id: str except Exception: return 0 - accepted: list[tuple[dict[str, Any], str, str]] = [] + accepted: list[tuple[dict[str, Any], str, str, str]] = [] for _ in range(max(0, scan_count)): try: event = completion_queue.get_nowait() @@ -140,52 +237,70 @@ def drain_ready_injects(agent: Any, messages: list[dict[str, Any]], turn_id: str completion_queue.put(event) continue - claim_id = claim_event_delivery(event, f"conversation-loop:{os.getpid()}") - if claim_id is None: - # A competing CLI/gateway process already owns this durable event, - # or it was delivered from a duplicate restored queue entry. - continue - + # Formatting is local preparation, not a delivery attempt. Do it before + # the durable claim so a broken spill/formatter cannot exhaust the + # bounded delivery-attempt budget without ever showing the result. try: text = _format_async_delegation(event) except Exception: logger.debug("Failed to format inject delegation event", exc_info=True) - release_event_delivery(event, claim_id) completion_queue.put(event) continue if not text: - release_event_delivery(event, claim_id) completion_queue.put(event) continue - accepted.append((event, claim_id, text)) + + claim_id = claim_event_delivery(event, f"conversation-loop:{os.getpid()}") + if claim_id is None: + # A competing CLI/gateway process already owns this durable event, + # or it was delivered from a duplicate restored queue entry. + continue + event_id = _event_identity(event) + if _durable_event_is_in_history(messages, event_id): + # A previous process persisted the synthetic message before it + # crashed. The active transcript is now the durable handoff. + complete_event_delivery(event, claim_id) + continue + accepted.append((event, claim_id, text, event_id)) if not accepted: return 0 content = "\n\n".join(item[2] for item in accepted) delegation_ids = [str(item[0].get("delegation_id") or "") for item in accepted] - event_ids = [ - f"{item[0].get('delegation_id') or ''}:{item[0].get('delivery_event_key') or 'aggregate'}" - for item in accepted - ] + event_ids = [item[3] for item in accepted] try: - messages.append( - { - "role": "user", - "content": content, - "_synthetic_delegation_inject": True, - "_delegation_ids": delegation_ids, - "_delegation_event_ids": event_ids, - } - ) + synthetic_message = { + "role": "user", + "content": content, + "display_kind": "delegation_inject", + "display_metadata": { + "delegation_ids": delegation_ids, + "delegation_event_ids": event_ids, + }, + "_synthetic_delegation_inject": True, + "_delegation_ids": delegation_ids, + "_delegation_event_ids": event_ids, + } + messages.append(synthetic_message) agent._session_messages = messages except Exception: - for event, claim_id, _text in accepted: + for event, claim_id, _text, _event_id in accepted: release_event_delivery(event, claim_id) completion_queue.put(event) raise - for event, claim_id, _text in accepted: - complete_event_delivery(event, claim_id) + pending = list(getattr(agent, _PENDING_CLAIMS_ATTR, []) or []) + pending.extend( + { + "event": event, + "claim_id": claim_id, + "event_id": event_id, + "message": synthetic_message, + "turn_id": str(turn_id), + } + for event, claim_id, _text, event_id in accepted + ) + setattr(agent, _PENDING_CLAIMS_ATTR, pending) _grant_reconciliation_grace_if_needed(agent, turn_id) return len(accepted) diff --git a/tests/agent/test_delegation_inject.py b/tests/agent/test_delegation_inject.py index ccb3687ffa2d..60546f3b9122 100644 --- a/tests/agent/test_delegation_inject.py +++ b/tests/agent/test_delegation_inject.py @@ -6,11 +6,17 @@ from types import SimpleNamespace import threading import time +from typing import Any import uuid import pytest -from agent.delegation_inject import drain_ready_injects, reconcile_provisional_final +from agent.delegation_inject import ( + acknowledge_pending_injects, + drain_ready_injects, + reconcile_provisional_final, + release_pending_injects, +) from tools import async_delegation as ad from tools import delegate_tool from tools.process_registry import process_registry @@ -86,6 +92,27 @@ def _event_state(delegation_id: str, event_key: str): ).fetchone() +def _parent_state(delegation_id: str): + with ad._DB_LOCK, ad._transaction() as conn: + return conn.execute( + "SELECT state, delivery_state FROM async_delegations " + "WHERE delegation_id=?", + (delegation_id,), + ).fetchone() + + +def _durable_event_keys(delegation_id: str): + with ad._DB_LOCK, ad._transaction() as conn: + return [ + row[0] + for row in conn.execute( + "SELECT event_key FROM async_delegation_events " + "WHERE delegation_id=? ORDER BY event_key", + (delegation_id,), + ).fetchall() + ] + + def test_inject_drain_rotates_unrelated_queue_items_and_coalesces_ready_children(): delegation_id = _record(goals=("audit A", "audit B")) unrelated = {"type": "completion", "session_id": "process-1"} @@ -103,9 +130,8 @@ def test_inject_drain_rotates_unrelated_queue_items_and_coalesces_ready_children {"role": "assistant", "tool_calls": [{"id": "tc1", "function": {}}]}, {"role": "tool", "tool_call_id": "tc1", "content": "done"}, ] - count = drain_ready_injects( - SimpleNamespace(_active_turn_id="turn-current"), messages, "turn-current" - ) + agent = SimpleNamespace(_active_turn_id="turn-current") + count = drain_ready_injects(agent, messages, "turn-current") assert count == 2 assert [m["role"] for m in messages] == ["assistant", "tool", "user"] @@ -118,6 +144,9 @@ def test_inject_drain_rotates_unrelated_queue_items_and_coalesces_ready_children f"{delegation_id}:task:1", ] assert _queue_contents() == [unrelated, legacy] + assert _event_state(delegation_id, "task:0") == ("pending", 1) + assert _event_state(delegation_id, "task:1") == ("pending", 1) + assert acknowledge_pending_injects(agent, turn_id="turn-current") == 2 assert _event_state(delegation_id, "task:0") == ("delivered", 1) assert _event_state(delegation_id, "task:1") == ("delivered", 1) @@ -130,10 +159,13 @@ def test_inject_drain_is_exactly_once_when_queue_contains_duplicate_event(): process_registry.completion_queue.put(deepcopy(event)) messages = [{"role": "tool", "tool_call_id": "tc", "content": "done"}] - assert drain_ready_injects(SimpleNamespace(), messages, "turn-current") == 1 + agent = SimpleNamespace() + assert drain_ready_injects(agent, messages, "turn-current") == 1 assert messages[-1]["content"].count("once") == 1 assert process_registry.completion_queue.empty() - assert drain_ready_injects(SimpleNamespace(), messages, "turn-current") == 0 + assert drain_ready_injects(agent, messages, "turn-current") == 0 + assert _event_state(delegation_id, "task:0") == ("pending", 1) + assert acknowledge_pending_injects(agent, turn_id="turn-current") == 1 assert _event_state(delegation_id, "task:0") == ("delivered", 1) @@ -343,6 +375,84 @@ def runner(): assert [r["summary"] for r in event["results"]] == ["A", "B"] +def test_inject_batch_finalization_rolls_back_children_if_parent_update_fails( + monkeypatch, +): + delegation_id = _record(goals=("A", "B")) + with ad._records_lock: + event_record = dict(ad._records[delegation_id]) + combined = {"results": [_child(0, "A"), _child(1, "B")]} + parent_event = { + **event_record, + "type": "async_delegation", + "status": "completed", + "completed_at": time.time(), + } + + def crash_before_parent_update(*_args, **_kwargs): + raise RuntimeError("simulated crash before parent terminal update") + + monkeypatch.setattr(ad, "_update_completion_row", crash_before_parent_update) + with pytest.raises(RuntimeError, match="simulated crash"): + ad._persist_inject_batch_finalization( + event_record, parent_event, combined, "completed" + ) + + assert _durable_event_keys(delegation_id) == [] + assert _parent_state(delegation_id) == ("running", "pending") + + +def test_recovery_with_complete_inject_children_suppresses_parent_aggregate(): + delegation_id = _record(goals=("A", "B")) + assert ad.publish_batch_child_completion(delegation_id, 0, _child(0, "A")) + assert ad.publish_batch_child_completion(delegation_id, 1, _child(1, "B")) + while not process_registry.completion_queue.empty(): + process_registry.completion_queue.get_nowait() + with ad._DB_LOCK, ad._transaction() as conn: + conn.execute( + "UPDATE async_delegations SET owner_pid=99999999, owner_started_at=0 " + "WHERE delegation_id=?", + (delegation_id,), + ) + + assert ad.recover_abandoned_delegations() == 1 + + assert _parent_state(delegation_id) == ("completed", "delivered") + assert _durable_event_keys(delegation_id) == ["task:0", "task:1"] + restored = __import__("queue").Queue() + assert ad.restore_undelivered_completions(restored) == 2 + assert {restored.get_nowait()["delivery_event_key"] for _ in range(2)} == { + "task:0", + "task:1", + } + + +def test_recovery_with_partial_inject_children_emits_only_terminal_gap_event(): + delegation_id = _record(goals=("A", "B")) + assert ad.publish_batch_child_completion(delegation_id, 0, _child(0, "A")) + process_registry.completion_queue.get_nowait() + with ad._DB_LOCK, ad._transaction() as conn: + conn.execute( + "UPDATE async_delegations SET owner_pid=99999999, owner_started_at=0 " + "WHERE delegation_id=?", + (delegation_id,), + ) + + assert ad.recover_abandoned_delegations() == 1 + + assert _parent_state(delegation_id) == ("unknown", "delivered") + assert _durable_event_keys(delegation_id) == ["task:0", "terminal"] + restored = __import__("queue").Queue() + assert ad.restore_undelivered_completions(restored) == 2 + events = [restored.get_nowait() for _ in range(2)] + assert {event["delivery_event_key"] for event in events} == { + "task:0", + "terminal", + } + terminal = next(event for event in events if event["delivery_event_key"] == "terminal") + assert "1/2 batch child results" in terminal["error"] + + def test_child_timeout_error_is_injectable_and_durable(): delegation_id = _record() assert ad.publish_batch_child_completion( @@ -351,12 +461,91 @@ def test_child_timeout_error_is_injectable_and_durable(): _child(0, "", status="timeout", error="child exceeded 10s"), ) messages = [{"role": "tool", "tool_call_id": "tc", "content": "done"}] - assert drain_ready_injects(SimpleNamespace(), messages, "turn-current") == 1 + agent = SimpleNamespace() + assert drain_ready_injects(agent, messages, "turn-current") == 1 assert "timeout" in messages[-1]["content"] assert "child exceeded 10s" in messages[-1]["content"] + assert _event_state(delegation_id, "task:0") == ("pending", 1) + assert acknowledge_pending_injects(agent, turn_id="turn-current") == 1 assert _event_state(delegation_id, "task:0") == ("delivered", 1) +def test_unconsumed_ram_inject_is_removed_released_and_requeued(): + delegation_id = _record() + assert ad.publish_batch_child_completion( + delegation_id, 0, _child(0, "retry after interrupt") + ) + original = {"role": "tool", "tool_call_id": "tc", "content": "done"} + messages = [original] + agent = SimpleNamespace() + + assert drain_ready_injects(agent, messages, "turn-current") == 1 + assert _event_state(delegation_id, "task:0") == ("pending", 1) + + assert release_pending_injects(agent, messages, turn_id="turn-current") == 1 + assert messages == [original] + assert [event["delegation_id"] for event in _queue_contents()] == [delegation_id] + assert _event_state(delegation_id, "task:0") == ("pending", 1) + + +def test_restart_dedups_inject_already_persisted_in_active_history(): + delegation_id = _record() + assert ad.publish_batch_child_completion( + delegation_id, 0, _child(0, "persisted before crash") + ) + messages: list[dict[str, Any]] = [ + {"role": "tool", "tool_call_id": "tc", "content": "done"} + ] + crashed_agent = SimpleNamespace() + + assert drain_ready_injects(crashed_agent, messages, "turn-current") == 1 + persisted = messages[-1] + persisted["_db_persisted"] = True + pending = crashed_agent._pending_delegation_inject_claims + crashed_event = deepcopy(pending[0]["event"]) + # Simulate process loss and expiry of the dead consumer's durable lease. + del crashed_agent._pending_delegation_inject_claims + with ad._DB_LOCK, ad._transaction() as conn: + conn.execute( + "UPDATE async_delegation_events SET delivery_claimed_at=0 " + "WHERE delegation_id=? AND event_key='task:0'", + (delegation_id,), + ) + process_registry.completion_queue.put(crashed_event) + + assert drain_ready_injects(SimpleNamespace(), messages, "turn-current") == 0 + assert messages[-1] is persisted + assert sum("persisted before crash" in m.get("content", "") for m in messages) == 1 + assert _event_state(delegation_id, "task:0") == ("pending", 1) + + # The resumed provider consumes the durable tail user message. At the next + # append-only boundary, the queued duplicate is acknowledged without append. + messages.append({"role": "assistant", "content": "consumed"}) + assert drain_ready_injects(SimpleNamespace(), messages, "turn-current") == 0 + assert sum("persisted before crash" in m.get("content", "") for m in messages) == 1 + assert _event_state(delegation_id, "task:0") == ("delivered", 2) + + +def test_formatter_failure_does_not_consume_delivery_attempts(monkeypatch): + delegation_id = _record() + assert ad.publish_batch_child_completion( + delegation_id, 0, _child(0, "format me") + ) + + def broken_formatter(_event): + raise ValueError("broken spill") + + monkeypatch.setattr( + "tools.process_registry._format_async_delegation", broken_formatter + ) + messages = [{"role": "tool", "tool_call_id": "tc", "content": "done"}] + for _ in range(ad._MAX_DELIVERY_ATTEMPTS + 2): + assert drain_ready_injects(SimpleNamespace(), messages, "turn-current") == 0 + + assert _event_state(delegation_id, "task:0") == ("pending", 0) + assert len(_queue_contents()) == 1 + + def test_pending_child_event_restores_with_same_durable_identity(tmp_path, monkeypatch): monkeypatch.setattr(ad, "_db_path", lambda: tmp_path / "state.db") delegation_id = _record(goals=("restore me",)) diff --git a/tools/async_delegation.py b/tools/async_delegation.py index e827ab467c5e..300562f59fdb 100644 --- a/tools/async_delegation.py +++ b/tools/async_delegation.py @@ -316,59 +316,63 @@ def _prune_durable_records() -> None: ) -def _persist_completion( +def _update_completion_row( + conn, event: Dict[str, Any], result: Dict[str, Any], *, delivery_state: str = "pending", + now: Optional[float] = None, ) -> None: - now = time.time() + now = time.time() if now is None else now state = "delivered" if delivery_state == "delivered" else "pending" delivered_at = now if state == "delivered" else None + conn.execute( + """UPDATE async_delegations SET state=?, completed_at=?, updated_at=?, + event_json=?, result_json=?, delivery_state=?, delivered_at=? + WHERE delegation_id=?""", + ( + event.get("status", "completed"), + event.get("completed_at", now), + now, + json.dumps(event), + json.dumps(result), + state, + delivered_at, + event["delegation_id"], + ), + ) + + +def _persist_completion( + event: Dict[str, Any], + result: Dict[str, Any], + *, + delivery_state: str = "pending", +) -> None: with _DB_LOCK, _transaction() as conn: - conn.execute( - """UPDATE async_delegations SET state=?, completed_at=?, updated_at=?, - event_json=?, result_json=?, delivery_state=?, delivered_at=? - WHERE delegation_id=?""", - ( - event.get("status", "completed"), - event.get("completed_at", now), - now, - json.dumps(event), - json.dumps(result), - state, - delivered_at, - event["delegation_id"], - ), + _update_completion_row( + conn, event, result, delivery_state=delivery_state ) -def publish_batch_child_completion( - delegation_id: str, +def _build_batch_child_event( + record: Dict[str, Any], task_index: int, result: Dict[str, Any], -) -> bool: - """Durably enqueue one ready child from an ``inject`` batch. - - The parent batch remains the execution/stall unit. This function only - creates an independently claimable delivery event, keyed by task index. - Repeated publication is idempotent and never resets a delivered claim. - """ - with _records_lock: - record = dict(_records.get(delegation_id) or {}) - if str(record.get("result_delivery") or "after_turn").lower() != "inject": - return False - + *, + completed_at: Optional[float] = None, +) -> Dict[str, Any]: + delegation_id = str(record.get("delegation_id") or "") goals = list(record.get("goals") or []) goal = goals[task_index] if 0 <= task_index < len(goals) else record.get("goal", "") - completed_at = time.time() + completed_at = time.time() if completed_at is None else completed_at child_result = dict(result or {}) child_result.setdefault("task_index", task_index) - event_key = f"task:{task_index}" - event = { + return { "type": "async_delegation", "delegation_id": delegation_id, - "delivery_event_key": event_key, + "delivery_event_key": f"task:{task_index}", "batch_id": delegation_id, "task_index": task_index, "batch_size": len(goals), @@ -399,16 +403,46 @@ def publish_batch_child_completion( "completed_at": completed_at, "result_delivery": "inject", } + + +def _insert_batch_event(conn, event: Dict[str, Any], *, now: float) -> bool: + cur = conn.execute( + """INSERT OR IGNORE INTO async_delegation_events + (delegation_id, event_key, event_json, delivery_state, + delivery_attempts, created_at, updated_at) + VALUES (?, ?, ?, 'pending', 0, ?, ?)""", + ( + event["delegation_id"], + event["delivery_event_key"], + json.dumps(event), + now, + now, + ), + ) + return cur.rowcount == 1 + + +def publish_batch_child_completion( + delegation_id: str, + task_index: int, + result: Dict[str, Any], +) -> bool: + """Durably enqueue one ready child from an ``inject`` batch. + + The parent batch remains the execution/stall unit. This function only + creates an independently claimable delivery event, keyed by task index. + Repeated publication is idempotent and never resets a delivered claim. + """ + with _records_lock: + record = dict(_records.get(delegation_id) or {}) + if str(record.get("result_delivery") or "after_turn").lower() != "inject": + return False + + event = _build_batch_child_event(record, task_index, result) now = time.time() with _DB_LOCK, _transaction() as conn: - cur = conn.execute( - """INSERT OR IGNORE INTO async_delegation_events - (delegation_id, event_key, event_json, delivery_state, - delivery_attempts, created_at, updated_at) - VALUES (?, ?, ?, 'pending', 0, ?, ?)""", - (delegation_id, event_key, json.dumps(event), now, now), - ) - if cur.rowcount != 1: + inserted = _insert_batch_event(conn, event, now=now) + if not inserted: return False from tools.process_registry import process_registry @@ -416,19 +450,23 @@ def publish_batch_child_completion( return True -def _publish_batch_terminal_event( - event_record: Dict[str, Any], combined: Dict[str, Any], status: str -) -> bool: - """Deliver a parent-level inject failure not represented by child events.""" - if status in {"completed", "success"} or (combined.get("results") or []): - return False +def _build_batch_terminal_event( + event_record: Dict[str, Any], + combined: Dict[str, Any], + status: str, + *, + force: bool = False, +) -> Optional[Dict[str, Any]]: + if not force and ( + status in {"completed", "success"} or (combined.get("results") or []) + ): + return None delegation_id = str(event_record.get("delegation_id") or "") - event_key = "terminal" now = time.time() - event = { + event: Dict[str, Any] = { "type": "async_delegation", "delegation_id": delegation_id, - "delivery_event_key": event_key, + "delivery_event_key": "terminal", "batch_id": delegation_id, "session_key": event_record.get("session_key", ""), "origin_ui_session_id": event_record.get("origin_ui_session_id", ""), @@ -457,15 +495,20 @@ def _publish_batch_terminal_event( ): if key in combined: event[key] = combined[key] + return event + + +def _publish_batch_terminal_event( + event_record: Dict[str, Any], combined: Dict[str, Any], status: str +) -> bool: + """Deliver a parent-level inject failure not represented by child events.""" + event = _build_batch_terminal_event(event_record, combined, status) + if event is None: + return False + now = time.time() with _DB_LOCK, _transaction() as conn: - cur = conn.execute( - """INSERT OR IGNORE INTO async_delegation_events - (delegation_id, event_key, event_json, delivery_state, - delivery_attempts, created_at, updated_at) - VALUES (?, ?, ?, 'pending', 0, ?, ?)""", - (delegation_id, event_key, json.dumps(event), now, now), - ) - if cur.rowcount != 1: + inserted = _insert_batch_event(conn, event, now=now) + if not inserted: return False from tools.process_registry import process_registry @@ -473,6 +516,52 @@ def _publish_batch_terminal_event( return True +def _persist_inject_batch_finalization( + event_record: Dict[str, Any], + parent_event: Dict[str, Any], + combined: Dict[str, Any], + status: str, +) -> None: + """Atomically persist missing child events and the terminal parent row.""" + + now = time.time() + newly_inserted: list[Dict[str, Any]] = [] + with _DB_LOCK, _transaction() as conn: + for child in combined.get("results") or []: + child_event = _build_batch_child_event( + event_record, + int(child.get("task_index", 0)), + child, + completed_at=now, + ) + if _insert_batch_event(conn, child_event, now=now): + newly_inserted.append(child_event) + + terminal_event = _build_batch_terminal_event( + event_record, combined, status + ) + if terminal_event is not None and _insert_batch_event( + conn, terminal_event, now=now + ): + newly_inserted.append(terminal_event) + + # The aggregate row is bookkeeping-only for inject batches. Commit its + # terminal state in the same transaction as the idempotent child safety + # net so restart recovery can never observe a half-finalized parent. + _update_completion_row( + conn, + parent_event, + combined, + delivery_state="delivered", + now=now, + ) + + from tools.process_registry import process_registry + + for queued_event in newly_inserted: + process_registry.completion_queue.put(queued_event) + + def _note_delivery_attempt(delegation_id: str) -> None: with _DB_LOCK, _transaction() as conn: conn.execute( @@ -508,6 +597,81 @@ def recover_abandoned_delegations() -> int: if live: continue task = json.loads(task_json or "{}") + restored_delivery = str( + result_delivery or task.get("result_delivery") or "after_turn" + ).lower() + if bool(task.get("is_batch")) and restored_delivery == "inject": + # Inject batches deliver child-scoped rows, never an aggregate. + # A crash after one or more child inserts must preserve that wire + # shape instead of manufacturing a contradictory parent unknown. + child_rows = conn.execute( + """SELECT event_json FROM async_delegation_events + WHERE delegation_id=? AND event_key LIKE 'task:%' + ORDER BY event_key""", + (delegation_id,), + ).fetchall() + child_results: list[Dict[str, Any]] = [] + for (child_payload,) in child_rows: + child_event = json.loads(child_payload or "{}") + results = child_event.get("results") or [] + if results and isinstance(results[0], dict): + child_results.append(results[0]) + + goals = list(task.get("goals") or []) + complete_children = bool(goals) and len(child_results) >= len(goals) + status = "completed" if complete_children else "unknown" + recovery_error = None + if not complete_children: + recovery_error = ( + "Delegation owner exited after recording " + f"{len(child_results)}/{len(goals)} batch child results; " + "remaining outcomes are unknown." + ) + combined = { + "results": child_results, + "error": recovery_error, + } + event_record = { + "delegation_id": delegation_id, + "session_key": session_key, + "origin_ui_session_id": origin_ui, + "origin_session_id": origin_session_id or "", + "parent_session_id": parent_id, + "parent_turn_id": task.get("parent_turn_id", ""), + "goal": task.get("goal", ""), + "goals": goals, + "context": task.get("context"), + "toolsets": task.get("toolsets"), + "role": task.get("role"), + "model": task.get("model"), + "dispatched_at": dispatched_at, + } + if not complete_children: + terminal_event = _build_batch_terminal_event( + event_record, combined, status, force=True + ) + if terminal_event is not None: + _insert_batch_event(conn, terminal_event, now=now) + + parent_event = { + **event_record, + "type": "async_delegation", + "status": status, + "is_batch": True, + "results": child_results, + "error": recovery_error, + "completed_at": now, + "result_delivery": "inject", + } + _update_completion_row( + conn, + parent_event, + combined, + delivery_state="delivered", + now=now, + ) + recovered += 1 + continue event = { "type": "async_delegation", "delegation_id": delegation_id, "session_key": session_key, "origin_ui_session_id": origin_ui, @@ -519,9 +683,7 @@ def recover_abandoned_delegations() -> int: "goals": task.get("goals"), "context": task.get("context"), "toolsets": task.get("toolsets"), "role": task.get("role"), "model": task.get("model"), "is_batch": bool(task.get("is_batch")), - "result_delivery": str( - result_delivery or task.get("result_delivery") or "after_turn" - ), + "result_delivery": restored_delivery, "status": "unknown", "summary": None, "error": "Delegation owner exited before recording a terminal result; outcome unknown.", "dispatched_at": dispatched_at, "completed_at": now, @@ -1389,20 +1551,13 @@ def _push_batch_completion_event( if _k in combined: evt[_k] = combined[_k] if str(event_record.get("result_delivery") or "after_turn").lower() == "inject": - # Child callbacks normally publish immediately. Re-publish here as an - # idempotent safety net for callback failures. Persist every child event - # before atomically acknowledging the aggregate parent row: otherwise a - # crash between a pending aggregate write and a separate acknowledgement - # could restore both the aggregate and its child events after restart. - delegation_id = str(event_record.get("delegation_id") or "") - for child in combined.get("results") or []: - publish_batch_child_completion( - delegation_id, - int(child.get("task_index", 0)), - child, - ) - _publish_batch_terminal_event(event_record, combined, status) - _persist_completion(evt, combined, delivery_state="delivered") + # Child callbacks publish incrementally for same-turn visibility. The + # finalizer idempotently inserts any missed child and commits the parent + # terminal row in one SQLite transaction; only newly inserted events are + # queued after that transaction commits. + _persist_inject_batch_finalization( + event_record, evt, combined, status + ) return _persist_completion(evt, combined) try: From 714275fa4e5aa121096d43d309a30eb446aefa07 Mon Sep 17 00:00:00 2001 From: Xipong <217837358+Xipong@users.noreply.github.com> Date: Thu, 30 Jul 2026 21:57:34 +0300 Subject: [PATCH 4/7] fix(delegation): keep inject claims alive through provider calls --- agent/conversation_loop.py | 21 +- agent/delegation_inject.py | 119 ++++++++-- tests/agent/test_delegation_inject.py | 301 ++++++++++++++++++++++++++ tools/async_delegation.py | 117 ++++++++-- 4 files changed, 524 insertions(+), 34 deletions(-) diff --git a/agent/conversation_loop.py b/agent/conversation_loop.py index 3935bbb76bb0..8e90ca759621 100644 --- a/agent/conversation_loop.py +++ b/agent/conversation_loop.py @@ -1259,14 +1259,28 @@ def run_conversation( should_review_memory=_should_review_memory, ) + def _release_unconsumed_injects() -> None: + try: + from agent.delegation_inject import release_pending_injects + + release_pending_injects(agent, messages, turn_id=turn_id) + except Exception: + logger.debug("Failed to settle unconsumed inject claims", exc_info=True) + while (api_call_count < agent.max_iterations and agent.iteration_budget.remaining > 0) or agent._budget_grace_call: # Safe boundary: the previous assistant tool-call block (if any) and # every corresponding tool result have already been appended. Drain # only already-ready results from this foreground turn. try: - from agent.delegation_inject import drain_ready_injects + from agent.delegation_inject import ( + drain_ready_injects, + ensure_pending_inject_heartbeat, + ) drain_ready_injects(agent, messages, turn_id) + # Start the lease before context assembly/compression; those steps + # can themselves be slow for very large parent histories. + ensure_pending_inject_heartbeat(agent) except Exception: logger.debug("Same-turn delegation inject drain failed", exc_info=True) @@ -5040,6 +5054,10 @@ def _perform_api_call(next_api_kwargs): force=True, ) logger.error(f"{agent.log_prefix}Non-retryable client error: {api_error}") + # The provider did not consume a normalized response. Remove + # RAM-only inject markers and release their durable claims + # before any terminal-error persistence can mark them saved. + _release_unconsumed_injects() # Skip session persistence when the error is likely # context-overflow related (status 400 + large session). # Persisting the failed user message would make the @@ -5250,6 +5268,7 @@ def _perform_api_call(next_api_kwargs): agent._dump_api_request_debug( api_kwargs, reason="max_retries_exhausted", error=api_error, ) + _release_unconsumed_injects() agent._persist_session(messages, conversation_history) _billing_block = None if classified.reason == FailoverReason.billing: diff --git a/agent/delegation_inject.py b/agent/delegation_inject.py index 334021a83bf7..c68e7e2266e2 100644 --- a/agent/delegation_inject.py +++ b/agent/delegation_inject.py @@ -10,6 +10,7 @@ import logging import os import queue +import threading from typing import Any logger = logging.getLogger(__name__) @@ -17,6 +18,8 @@ _GRACE_TURN_ATTR = "_delegation_reconciliation_grace_turn_id" _PENDING_CLAIMS_ATTR = "_pending_delegation_inject_claims" +_CLAIM_HEARTBEAT_ATTR = "_delegation_inject_claim_heartbeat" +_CLAIM_HEARTBEAT_INTERVAL_SECONDS = 60.0 def _event_identity(event: dict[str, Any]) -> str: @@ -46,6 +49,66 @@ def _durable_event_is_in_history( ) +def _stop_claim_heartbeat_if_idle(agent: Any) -> None: + if getattr(agent, _PENDING_CLAIMS_ATTR, None): + return + heartbeat = getattr(agent, _CLAIM_HEARTBEAT_ATTR, None) + if isinstance(heartbeat, dict): + stop = heartbeat.get("stop") + if isinstance(stop, threading.Event): + stop.set() + + +def ensure_pending_inject_heartbeat(agent: Any) -> bool: + """Renew live same-turn claims throughout provider retries and backoff.""" + + if not getattr(agent, _PENDING_CLAIMS_ATTR, None): + return False + existing = getattr(agent, _CLAIM_HEARTBEAT_ATTR, None) + if isinstance(existing, dict): + thread = existing.get("thread") + existing_stop = existing.get("stop") + if ( + isinstance(thread, threading.Thread) + and thread.is_alive() + and isinstance(existing_stop, threading.Event) + and not existing_stop.is_set() + ): + return True + + stop = threading.Event() + + def _heartbeat() -> None: + from tools.async_delegation import renew_event_delivery + + while not stop.wait(_CLAIM_HEARTBEAT_INTERVAL_SECONDS): + pending = list(getattr(agent, _PENDING_CLAIMS_ATTR, []) or []) + if not pending: + break + for entry in pending: + try: + if not renew_event_delivery(entry["event"], entry["claim_id"]): + logger.warning( + "Could not renew same-turn delegation claim %s", + entry.get("event_id"), + ) + except Exception: + logger.warning( + "Failed to renew same-turn delegation claim %s", + entry.get("event_id"), + exc_info=True, + ) + + thread = threading.Thread( + target=_heartbeat, + daemon=True, + name="delegation-inject-claim-heartbeat", + ) + setattr(agent, _CLAIM_HEARTBEAT_ATTR, {"stop": stop, "thread": thread}) + thread.start() + return True + + def acknowledge_pending_injects(agent: Any, *, turn_id: str | None = None) -> int: """Acknowledge inject claims after a provider consumed their message.""" @@ -58,9 +121,16 @@ def acknowledge_pending_injects(agent: Any, *, turn_id: str | None = None) -> in if turn_id is not None and str(entry.get("turn_id") or "") != str(turn_id): keep.append(entry) continue - complete_event_delivery(entry["event"], entry["claim_id"]) - acknowledged += 1 + if complete_event_delivery(entry["event"], entry["claim_id"]): + acknowledged += 1 + else: + keep.append(entry) + logger.warning( + "Provider consumed delegation inject %s but durable ack did not commit", + entry.get("event_id"), + ) setattr(agent, _PENDING_CLAIMS_ATTR, keep) + _stop_claim_heartbeat_if_idle(agent) return acknowledged @@ -74,14 +144,15 @@ def release_pending_injects( from tools.async_delegation import ( complete_event_delivery, + get_event_delivery_state, release_event_delivery, ) from tools.process_registry import process_registry pending = list(getattr(agent, _PENDING_CLAIMS_ATTR, []) or []) keep: list[dict[str, Any]] = [] - removable_message_ids: set[int] = set() - released = 0 + removable_event_ids: set[str] = set() + settled = 0 for entry in pending: if turn_id is not None and str(entry.get("turn_id") or "") != str(turn_id): keep.append(entry) @@ -89,20 +160,36 @@ def release_pending_injects( event = entry["event"] event_id = str(entry["event_id"]) if _durable_event_is_in_history(messages, event_id): - complete_event_delivery(event, entry["claim_id"]) + if complete_event_delivery(event, entry["claim_id"]): + settled += 1 + else: + keep.append(entry) else: - release_event_delivery(event, entry["claim_id"]) - process_registry.completion_queue.put(event) - removable_message_ids.add(id(entry["message"])) - released += 1 - - if removable_message_ids: + # Remove the unconsumed marker by durable identity even when + # compression replaced the Python dict object. + removable_event_ids.add(event_id) + committed = release_event_delivery(event, entry["claim_id"]) + state = get_event_delivery_state(event) + if committed: + # At the attempt cap release transitions to dropped, not pending. + if state == "pending": + process_registry.completion_queue.put(event) + settled += 1 + elif state == "delivered": + settled += 1 + else: + keep.append(entry) + + if removable_event_ids: messages[:] = [ - message for message in messages if id(message) not in removable_message_ids + message + for message in messages + if not (_message_event_ids(message) & removable_event_ids) ] agent._session_messages = messages setattr(agent, _PENDING_CLAIMS_ATTR, keep) - return released + _stop_claim_heartbeat_if_idle(agent) + return settled def _normal_budget_available(agent: Any) -> bool: @@ -208,6 +295,7 @@ def drain_ready_injects(agent: Any, messages: list[dict[str, Any]], turn_id: str from tools.async_delegation import ( claim_event_delivery, complete_event_delivery, + get_event_delivery_state, release_event_delivery, ) from tools.process_registry import _format_async_delegation, process_registry @@ -286,8 +374,9 @@ def drain_ready_injects(agent: Any, messages: list[dict[str, Any]], turn_id: str agent._session_messages = messages except Exception: for event, claim_id, _text, _event_id in accepted: - release_event_delivery(event, claim_id) - completion_queue.put(event) + if release_event_delivery(event, claim_id): + if get_event_delivery_state(event) == "pending": + completion_queue.put(event) raise pending = list(getattr(agent, _PENDING_CLAIMS_ATTR, []) or []) diff --git a/tests/agent/test_delegation_inject.py b/tests/agent/test_delegation_inject.py index 60546f3b9122..c8b0bbd09e49 100644 --- a/tests/agent/test_delegation_inject.py +++ b/tests/agent/test_delegation_inject.py @@ -2,15 +2,20 @@ from __future__ import annotations +import contextlib from copy import deepcopy +import io +from pathlib import Path from types import SimpleNamespace import threading import time from typing import Any import uuid +from unittest.mock import MagicMock, patch import pytest +from agent import delegation_inject as inject from agent.delegation_inject import ( acknowledge_pending_injects, drain_ready_injects, @@ -20,6 +25,8 @@ from tools import async_delegation as ad from tools import delegate_tool from tools.process_registry import process_registry +from hermes_state import SessionDB +from run_agent import AIAgent @pytest.fixture(autouse=True) @@ -113,6 +120,61 @@ def _durable_event_keys(delegation_id: str): ] +def _loop_response(*, content, finish_reason="stop", tool_calls=None): + message = SimpleNamespace(content=content, tool_calls=tool_calls) + choice = SimpleNamespace(message=message, finish_reason=finish_reason) + return SimpleNamespace(choices=[choice], model="test/model", usage=None) + + +def _loop_tool_call(): + return SimpleNamespace( + id="call-inject", + type="function", + function=SimpleNamespace(name="terminal", arguments="{}"), + ) + + +def _make_loop_agent(tmp_path: Path) -> AIAgent: + tool_defs = [ + { + "type": "function", + "function": { + "name": "terminal", + "description": "test boundary", + "parameters": {"type": "object", "properties": {}}, + }, + } + ] + with ( + contextlib.redirect_stdout(io.StringIO()), + patch("run_agent.get_tool_definitions", return_value=tool_defs), + patch("run_agent.check_toolset_requirements", return_value={}), + patch("run_agent.OpenAI"), + ): + agent = AIAgent( + api_key="test-key", + base_url="https://example.invalid/v1", + provider="openai", + api_mode="chat_completions", + model="test/model", + quiet_mode=True, + max_iterations=4, + skip_context_files=True, + skip_memory=True, + session_db=SessionDB(db_path=tmp_path / "state.db"), + session_id=f"inject-loop-{uuid.uuid4().hex}", + ) + agent.client = MagicMock() + agent._cached_system_prompt = "You are helpful." + agent._use_prompt_caching = False + agent._disable_streaming = True + agent.compression_enabled = False + agent.save_trajectories = False + agent.tool_delay = 0 + agent.valid_tool_names = {"terminal"} + return agent + + def test_inject_drain_rotates_unrelated_queue_items_and_coalesces_ready_children(): delegation_id = _record(goals=("audit A", "audit B")) unrelated = {"type": "completion", "session_id": "process-1"} @@ -453,6 +515,27 @@ def test_recovery_with_partial_inject_children_emits_only_terminal_gap_event(): assert "1/2 batch child results" in terminal["error"] +def test_recovery_requires_exact_expected_batch_child_keys(): + delegation_id = _record(goals=("A", "B")) + assert ad.publish_batch_child_completion(delegation_id, 0, _child(0, "A")) + process_registry.completion_queue.get_nowait() + with ad._records_lock: + event_record = dict(ad._records[delegation_id]) + rogue = ad._build_batch_child_event(event_record, 99, _child(99, "rogue")) + with ad._DB_LOCK, ad._transaction() as conn: + assert ad._insert_batch_event(conn, rogue, now=time.time()) + conn.execute( + "UPDATE async_delegations SET owner_pid=99999999, owner_started_at=0 " + "WHERE delegation_id=?", + (delegation_id,), + ) + + assert ad.recover_abandoned_delegations() == 1 + + assert _parent_state(delegation_id) == ("unknown", "delivered") + assert _durable_event_keys(delegation_id) == ["task:0", "task:99", "terminal"] + + def test_child_timeout_error_is_injectable_and_durable(): delegation_id = _record() assert ad.publish_batch_child_completion( @@ -488,6 +571,224 @@ def test_unconsumed_ram_inject_is_removed_released_and_requeued(): assert _event_state(delegation_id, "task:0") == ("pending", 1) +def test_compression_copy_is_removed_by_durable_event_identity_on_release(): + delegation_id = _record() + assert ad.publish_batch_child_completion( + delegation_id, 0, _child(0, "copy-safe rollback") + ) + original = {"role": "tool", "tool_call_id": "tc", "content": "done"} + messages = [original] + agent = SimpleNamespace() + + assert drain_ready_injects(agent, messages, "turn-current") == 1 + messages[:] = deepcopy(messages) + assert messages[-1] is not agent._pending_delegation_inject_claims[0]["message"] + + assert release_pending_injects(agent, messages, turn_id="turn-current") == 1 + assert messages == [original] + assert [event["delegation_id"] for event in _queue_contents()] == [delegation_id] + + +def test_failed_ack_keeps_ram_claim_for_later_reconciliation(monkeypatch): + delegation_id = _record() + assert ad.publish_batch_child_completion( + delegation_id, 0, _child(0, "ack must commit") + ) + messages = [{"role": "tool", "tool_call_id": "tc", "content": "done"}] + agent = SimpleNamespace() + assert drain_ready_injects(agent, messages, "turn-current") == 1 + + monkeypatch.setattr(ad, "complete_event_delivery", lambda *_args: False) + assert acknowledge_pending_injects(agent, turn_id="turn-current") == 0 + assert len(agent._pending_delegation_inject_claims) == 1 + assert _event_state(delegation_id, "task:0") == ("pending", 1) + + +def test_failed_release_neither_requeues_nor_forgets_claim(monkeypatch): + delegation_id = _record() + assert ad.publish_batch_child_completion( + delegation_id, 0, _child(0, "release must commit") + ) + original = {"role": "tool", "tool_call_id": "tc", "content": "done"} + messages = [original] + agent = SimpleNamespace() + assert drain_ready_injects(agent, messages, "turn-current") == 1 + + monkeypatch.setattr(ad, "release_event_delivery", lambda *_args: False) + assert release_pending_injects(agent, messages, turn_id="turn-current") == 0 + assert messages == [original] + assert process_registry.completion_queue.empty() + assert len(agent._pending_delegation_inject_claims) == 1 + + +def test_durable_claim_renewal_prevents_expiry_steal(): + delegation_id = _record() + assert ad.publish_batch_child_completion( + delegation_id, 0, _child(0, "renew durable lease") + ) + event = process_registry.completion_queue.get_nowait() + claim = ad.claim_event_delivery(event, "slow-main") + assert claim + with ad._DB_LOCK, ad._transaction() as conn: + conn.execute( + "UPDATE async_delegation_events SET delivery_claimed_at=0 " + "WHERE delegation_id=? AND event_key='task:0'", + (delegation_id,), + ) + + assert ad.renew_event_delivery(event, claim) is True + assert ad.claim_event_delivery(event, "competing-main") is None + assert ad.complete_event_delivery(event, claim) is True + + +def test_pending_claim_heartbeat_renews_until_ack(monkeypatch): + delegation_id = _record() + assert ad.publish_batch_child_completion( + delegation_id, 0, _child(0, "slow provider") + ) + renewed = threading.Event() + + def fake_renew(_event, _claim_id): + renewed.set() + return True + + monkeypatch.setattr(ad, "renew_event_delivery", fake_renew, raising=False) + monkeypatch.setattr( + inject, "_CLAIM_HEARTBEAT_INTERVAL_SECONDS", 0.01, raising=False + ) + messages = [{"role": "tool", "tool_call_id": "tc", "content": "done"}] + agent = SimpleNamespace() + + assert drain_ready_injects(agent, messages, "turn-current") == 1 + assert inject.ensure_pending_inject_heartbeat(agent) is True + assert renewed.wait(timeout=1), "pending inject claim was not renewed" + assert acknowledge_pending_injects(agent, turn_id="turn-current") == 1 + heartbeat = agent._delegation_inject_claim_heartbeat + heartbeat["thread"].join(timeout=1) + assert not heartbeat["thread"].is_alive() + + +def test_run_conversation_inject_transport_normalize_and_ack(monkeypatch, tmp_path): + agent = _make_loop_agent(tmp_path) + requests = [] + responses = [ + _loop_response( + content="", + finish_reason="tool_calls", + tool_calls=[_loop_tool_call()], + ), + _loop_response(content="model consumed LIVE_LOOP_INJECT"), + ] + + def create(**kwargs): + requests.append(deepcopy(kwargs["messages"])) + return responses.pop(0) + + agent.client.chat.completions.create.side_effect = create + published = {} + + def handle_tool(*_args, **_kwargs): + delegation_id = _record(turn_id=str(agent._active_turn_id)) + published["delegation_id"] = delegation_id + assert ad.publish_batch_child_completion( + delegation_id, 0, _child(0, "LIVE_LOOP_INJECT") + ) + return "tool boundary complete" + + with ( + patch("run_agent.handle_function_call", side_effect=handle_tool), + patch.object(agent, "_persist_session"), + patch.object(agent, "_save_trajectory"), + patch.object(agent, "_cleanup_task_resources"), + ): + result = agent.run_conversation("exercise inject lifecycle") + + delegation_id = published["delegation_id"] + assert result["completed"] is True + assert len(requests) == 2 + assert "LIVE_LOOP_INJECT" in str(requests[1]) + assert _event_state(delegation_id, "task:0") == ("delivered", 1) + assert not agent._pending_delegation_inject_claims + heartbeat = agent._delegation_inject_claim_heartbeat + heartbeat["thread"].join(timeout=1) + assert not heartbeat["thread"].is_alive() + + +def test_run_conversation_compression_copy_then_provider_error_releases( + monkeypatch, tmp_path +): + agent = _make_loop_agent(tmp_path) + calls = {"provider": 0, "compress": 0, "marker_compress": 0} + provider_error = RuntimeError("provider rejected request") + provider_error.status_code = 400 + + def create(**_kwargs): + calls["provider"] += 1 + if calls["provider"] == 1: + return _loop_response( + content="", + finish_reason="tool_calls", + tool_calls=[_loop_tool_call()], + ) + raise provider_error + + agent.client.chat.completions.create.side_effect = create + published = {} + + def handle_tool(*_args, **_kwargs): + delegation_id = _record(turn_id=str(agent._active_turn_id)) + published["delegation_id"] = delegation_id + assert ad.publish_batch_child_completion( + delegation_id, 0, _child(0, "COPY_THEN_RELEASE") + ) + agent.compression_enabled = True + agent.context_compressor.should_compress = lambda _tokens: True + return "tool boundary complete" + + def copy_compress(messages, system_message, **_kwargs): + calls["compress"] += 1 + if "COPY_THEN_RELEASE" not in str(messages): + # The post-tool compression boundary runs before the next loop-top + # drain. Only the following pre-API pass exercises copied injects. + return messages, system_message + calls["marker_compress"] += 1 + heartbeat = agent._delegation_inject_claim_heartbeat + assert heartbeat["thread"].is_alive() + assert not heartbeat["stop"].is_set() + agent.compression_enabled = False + return deepcopy(messages), system_message + + def persist_without_unconsumed_inject(messages, *_args, **_kwargs): + assert "COPY_THEN_RELEASE" not in str(messages) + + with ( + patch("run_agent.handle_function_call", side_effect=handle_tool), + patch.object(agent, "_compress_context", side_effect=copy_compress), + patch.object( + agent, + "_persist_session", + side_effect=persist_without_unconsumed_inject, + ), + patch.object(agent, "_save_trajectory"), + patch.object(agent, "_cleanup_task_resources"), + ): + result = agent.run_conversation("exercise rollback lifecycle") + + delegation_id = published["delegation_id"] + assert result["failed"] is True + assert calls["compress"] >= 2 + assert calls["marker_compress"] == 1 + assert "COPY_THEN_RELEASE" not in str(agent._session_messages) + assert _event_state(delegation_id, "task:0") == ("pending", 1) + assert any( + event.get("delegation_id") == delegation_id for event in _queue_contents() + ) + assert not agent._pending_delegation_inject_claims + heartbeat = agent._delegation_inject_claim_heartbeat + heartbeat["thread"].join(timeout=1) + assert not heartbeat["thread"].is_alive() + + def test_restart_dedups_inject_already_persisted_in_active_history(): delegation_id = _record() assert ad.publish_batch_child_completion( diff --git a/tools/async_delegation.py b/tools/async_delegation.py index 300562f59fdb..99ebcc8e2884 100644 --- a/tools/async_delegation.py +++ b/tools/async_delegation.py @@ -84,6 +84,10 @@ # attempts so an unroutable row converges to a terminal 'dropped' state # instead of replaying on every restart forever. _MAX_DELIVERY_ATTEMPTS = 8 +# Durable claims are leases, not ownership transfers. Active same-turn injects +# renew this timestamp while the provider request/retry lifecycle is alive; +# after a process crash the absent heartbeat lets another consumer recover it. +_DELIVERY_CLAIM_LEASE_SECONDS = 300 _DB_LOCK = threading.Lock() # --------------------------------------------------------------------------- @@ -605,20 +609,30 @@ def recover_abandoned_delegations() -> int: # A crash after one or more child inserts must preserve that wire # shape instead of manufacturing a contradictory parent unknown. child_rows = conn.execute( - """SELECT event_json FROM async_delegation_events + """SELECT event_key, event_json FROM async_delegation_events WHERE delegation_id=? AND event_key LIKE 'task:%' ORDER BY event_key""", (delegation_id,), ).fetchall() - child_results: list[Dict[str, Any]] = [] - for (child_payload,) in child_rows: + goals = list(task.get("goals") or []) + expected_child_keys = [f"task:{index}" for index in range(len(goals))] + child_results_by_key: dict[str, Dict[str, Any]] = {} + for event_key, child_payload in child_rows: + if event_key not in expected_child_keys: + continue child_event = json.loads(child_payload or "{}") results = child_event.get("results") or [] if results and isinstance(results[0], dict): - child_results.append(results[0]) - - goals = list(task.get("goals") or []) - complete_children = bool(goals) and len(child_results) >= len(goals) + child_results_by_key[event_key] = results[0] + + child_results = [ + child_results_by_key[key] + for key in expected_child_keys + if key in child_results_by_key + ] + complete_children = bool(expected_child_keys) and all( + key in child_results_by_key for key in expected_child_keys + ) status = "completed" if complete_children else "unknown" recovery_error = None if not complete_children: @@ -769,7 +783,13 @@ def claim_completion_delivery(delegation_id: str, claim_id: str) -> bool: delivery_attempts=delivery_attempts+1, updated_at=? WHERE delegation_id=? AND delivery_state='pending' AND (delivery_claim IS NULL OR delivery_claimed_at < ?)""", - (claim_id, now, now, delegation_id, now - 300), + ( + claim_id, + now, + now, + delegation_id, + now - _DELIVERY_CLAIM_LEASE_SECONDS, + ), ) return cur.rowcount == 1 @@ -784,7 +804,14 @@ def _claim_child_event(delegation_id: str, event_key: str, claim_id: str) -> boo WHERE delegation_id=? AND event_key=? AND delivery_state='pending' AND (delivery_claim IS NULL OR delivery_claimed_at < ?)""", - (claim_id, now, now, delegation_id, event_key, now - 300), + ( + claim_id, + now, + now, + delegation_id, + event_key, + now - _DELIVERY_CLAIM_LEASE_SECONDS, + ), ) return cur.rowcount == 1 @@ -805,6 +832,60 @@ def claim_event_delivery(evt: Dict[str, Any], consumer: str) -> Optional[str]: return claim_id if claimed else None +def renew_event_delivery(evt: Dict[str, Any], claim_id: str) -> bool: + """Renew the lease held by the exact consumer claim.""" + + if not claim_id or evt.get("type") != "async_delegation": + return False + delegation_id = str(evt.get("delegation_id") or "") + if not delegation_id: + return False + event_key = str(evt.get("delivery_event_key") or "") + now = time.time() + with _DB_LOCK, _transaction() as conn: + if event_key: + cur = conn.execute( + """UPDATE async_delegation_events + SET delivery_claimed_at=?, updated_at=? + WHERE delegation_id=? AND event_key=? + AND delivery_state='pending' AND delivery_claim=?""", + (now, now, delegation_id, event_key, claim_id), + ) + else: + cur = conn.execute( + """UPDATE async_delegations + SET delivery_claimed_at=?, updated_at=? + WHERE delegation_id=? AND delivery_state='pending' + AND delivery_claim=?""", + (now, now, delegation_id, claim_id), + ) + return cur.rowcount == 1 + + +def get_event_delivery_state(evt: Dict[str, Any]) -> Optional[str]: + """Return the durable state for one aggregate or child event.""" + + if evt.get("type") != "async_delegation": + return None + delegation_id = str(evt.get("delegation_id") or "") + if not delegation_id: + return None + event_key = str(evt.get("delivery_event_key") or "") + with _DB_LOCK, _transaction() as conn: + if event_key: + row = conn.execute( + """SELECT delivery_state FROM async_delegation_events + WHERE delegation_id=? AND event_key=?""", + (delegation_id, event_key), + ).fetchone() + else: + row = conn.execute( + "SELECT delivery_state FROM async_delegations WHERE delegation_id=?", + (delegation_id,), + ).fetchone() + return str(row[0]) if row is not None else None + + def release_completion_delivery(delegation_id: str, claim_id: str) -> bool: """Release a failed delivery claim so another consumer may retry. @@ -918,26 +999,26 @@ def _release_child_event(delegation_id: str, event_key: str, claim_id: str) -> b return cur.rowcount == 1 -def complete_event_delivery(evt: Dict[str, Any], claim_id: str) -> None: +def complete_event_delivery(evt: Dict[str, Any], claim_id: str) -> bool: if not claim_id or evt.get("type") != "async_delegation": - return + return False delegation_id = str(evt.get("delegation_id") or "") event_key = str(evt.get("delivery_event_key") or "") if event_key: - _complete_child_event(delegation_id, event_key, claim_id) + completed = _complete_child_event(delegation_id, event_key, claim_id) else: - complete_completion_delivery(delegation_id, claim_id) + completed = complete_completion_delivery(delegation_id, claim_id) + return completed or get_event_delivery_state(evt) == "delivered" -def release_event_delivery(evt: Dict[str, Any], claim_id: str) -> None: +def release_event_delivery(evt: Dict[str, Any], claim_id: str) -> bool: if not claim_id or evt.get("type") != "async_delegation": - return + return False delegation_id = str(evt.get("delegation_id") or "") event_key = str(evt.get("delivery_event_key") or "") if event_key: - _release_child_event(delegation_id, event_key, claim_id) - else: - release_completion_delivery(delegation_id, claim_id) + return _release_child_event(delegation_id, event_key, claim_id) + return release_completion_delivery(delegation_id, claim_id) def drop_event_delivery(evt: Dict[str, Any], claim_id: str) -> None: From 177d2505a64bf77bb6804a6b1cc926e05752a1fb Mon Sep 17 00:00:00 2001 From: Xipong <217837358+Xipong@users.noreply.github.com> Date: Thu, 30 Jul 2026 23:10:28 +0300 Subject: [PATCH 5/7] fix(tui): reserve same-turn delegation injects --- agent/delegation_inject.py | 93 ++++---- tests/agent/test_delegation_inject.py | 3 + tests/test_tui_gateway_server.py | 150 +++++++++++++ tools/process_registry.py | 7 + tui_gateway/server.py | 291 +++++++++++++++----------- 5 files changed, 378 insertions(+), 166 deletions(-) diff --git a/agent/delegation_inject.py b/agent/delegation_inject.py index c68e7e2266e2..4ee9c5656106 100644 --- a/agent/delegation_inject.py +++ b/agent/delegation_inject.py @@ -301,55 +301,60 @@ def drain_ready_injects(agent: Any, messages: list[dict[str, Any]], turn_id: str from tools.process_registry import _format_async_delegation, process_registry completion_queue = process_registry.completion_queue - try: - scan_count = completion_queue.qsize() - except Exception: - return 0 - accepted: list[tuple[dict[str, Any], str, str, str]] = [] - for _ in range(max(0, scan_count)): + # Atomic with TUI dequeue -> route/requeue/claim. Without this guard the + # TUI poller can temporarily hold a ready event outside the queue while this + # bounded snapshot reports zero, nondeterministically degrading inject to a + # later synthetic turn. + with process_registry.completion_routing_lock: try: - event = completion_queue.get_nowait() - except queue.Empty: - break + scan_count = completion_queue.qsize() except Exception: - break + return 0 - delivery = str(event.get("result_delivery") or "after_turn").strip().lower() - event_turn_id = str(event.get("parent_turn_id") or "") - if ( - event.get("type") != "async_delegation" - or delivery != "inject" - or event_turn_id != str(turn_id) - ): - completion_queue.put(event) - continue - - # Formatting is local preparation, not a delivery attempt. Do it before - # the durable claim so a broken spill/formatter cannot exhaust the - # bounded delivery-attempt budget without ever showing the result. - try: - text = _format_async_delegation(event) - except Exception: - logger.debug("Failed to format inject delegation event", exc_info=True) - completion_queue.put(event) - continue - if not text: - completion_queue.put(event) - continue + for _ in range(max(0, scan_count)): + try: + event = completion_queue.get_nowait() + except queue.Empty: + break + except Exception: + break - claim_id = claim_event_delivery(event, f"conversation-loop:{os.getpid()}") - if claim_id is None: - # A competing CLI/gateway process already owns this durable event, - # or it was delivered from a duplicate restored queue entry. - continue - event_id = _event_identity(event) - if _durable_event_is_in_history(messages, event_id): - # A previous process persisted the synthetic message before it - # crashed. The active transcript is now the durable handoff. - complete_event_delivery(event, claim_id) - continue - accepted.append((event, claim_id, text, event_id)) + delivery = str(event.get("result_delivery") or "after_turn").strip().lower() + event_turn_id = str(event.get("parent_turn_id") or "") + if ( + event.get("type") != "async_delegation" + or delivery != "inject" + or event_turn_id != str(turn_id) + ): + completion_queue.put(event) + continue + + # Formatting is local preparation, not a delivery attempt. Do it before + # the durable claim so a broken spill/formatter cannot exhaust the + # bounded delivery-attempt budget without ever showing the result. + try: + text = _format_async_delegation(event) + except Exception: + logger.debug("Failed to format inject delegation event", exc_info=True) + completion_queue.put(event) + continue + if not text: + completion_queue.put(event) + continue + + claim_id = claim_event_delivery(event, f"conversation-loop:{os.getpid()}") + if claim_id is None: + # A competing CLI/gateway process already owns this durable event, + # or it was delivered from a duplicate restored queue entry. + continue + event_id = _event_identity(event) + if _durable_event_is_in_history(messages, event_id): + # A previous process persisted the synthetic message before it + # crashed. The active transcript is now the durable handoff. + complete_event_delivery(event, claim_id) + continue + accepted.append((event, claim_id, text, event_id)) if not accepted: return 0 diff --git a/tests/agent/test_delegation_inject.py b/tests/agent/test_delegation_inject.py index c8b0bbd09e49..63fbe329acf0 100644 --- a/tests/agent/test_delegation_inject.py +++ b/tests/agent/test_delegation_inject.py @@ -670,6 +670,7 @@ def fake_renew(_event, _claim_id): def test_run_conversation_inject_transport_normalize_and_ack(monkeypatch, tmp_path): agent = _make_loop_agent(tmp_path) + cached_system_prompt = deepcopy(getattr(agent, "_cached_system_prompt")) requests = [] responses = [ _loop_response( @@ -706,6 +707,8 @@ def handle_tool(*_args, **_kwargs): delegation_id = published["delegation_id"] assert result["completed"] is True assert len(requests) == 2 + assert requests[1][: len(requests[0])] == requests[0] + assert getattr(agent, "_cached_system_prompt") == cached_system_prompt assert "LIVE_LOOP_INJECT" in str(requests[1]) assert _event_state(delegation_id, "task:0") == ("delivered", 1) assert not agent._pending_delegation_inject_claims diff --git a/tests/test_tui_gateway_server.py b/tests/test_tui_gateway_server.py index ada479fafbbf..1e4d713f212b 100644 --- a/tests/test_tui_gateway_server.py +++ b/tests/test_tui_gateway_server.py @@ -13121,6 +13121,156 @@ def test_notification_poller_requeues_when_busy(monkeypatch): process_registry.completion_queue.get_nowait() +def test_busy_tui_poller_cannot_hide_inject_from_active_loop(monkeypatch): + """A dequeued/requeued TUI event remains atomic with same-turn draining.""" + import queue as _queue_mod + + import agent.delegation_inject as inject_mod + import tools.async_delegation as delegation_mod + import tools.process_registry as registry_mod + from tools.process_registry import process_registry + + dequeued = threading.Event() + release_dequeue = threading.Event() + stop_poller = threading.Event() + + class _PausedAfterDequeueQueue(_queue_mod.Queue): + def get(self, block=True, timeout=None): + event = super().get(block=block, timeout=timeout) + dequeued.set() + if not release_dequeue.wait(3): + raise TimeoutError("test did not release paused TUI dequeue") + stop_poller.set() + return event + + isolated_queue = _PausedAfterDequeueQueue() + monkeypatch.setattr(process_registry, "completion_queue", isolated_queue) + monkeypatch.setattr(registry_mod, "format_process_notification", lambda _evt: "ready") + monkeypatch.setattr(registry_mod, "_format_async_delegation", lambda _evt: "ready") + monkeypatch.setattr(inject_mod, "ensure_pending_inject_heartbeat", lambda _agent: True) + + claims = [] + + def _claim(event, owner): + claims.append((event["delivery_event_key"], owner)) + return "claim-active-loop" + + monkeypatch.setattr(delegation_mod, "claim_event_delivery", _claim) + + turn_id = "turn-tui-inject" + sid = "sid-tui-inject" + agent = types.SimpleNamespace(_active_turn_id=turn_id) + sess = _session( + agent=agent, + running=True, + session_key="session-tui-inject", + ) + event = { + "type": "async_delegation", + "delegation_id": "deleg-tui-inject", + "delivery_event_key": "task:0", + "result_delivery": "inject", + "parent_turn_id": turn_id, + "origin_ui_session_id": sid, + "session_key": "session-tui-inject", + } + isolated_queue.put(event) + server._sessions[sid] = sess + monkeypatch.setattr(server, "_emit", lambda *_args, **_kwargs: None) + + messages = [{"role": "assistant", "content": "working"}] + drained = {} + drain_done = threading.Event() + + def _drain(): + drained["count"] = inject_mod.drain_ready_injects(agent, messages, turn_id) + drain_done.set() + + poller_thread = threading.Thread( + target=server._notification_poller_loop, + args=(stop_poller, sid, sess), + ) + drain_thread = threading.Thread(target=_drain) + + try: + poller_thread.start() + assert dequeued.wait(3), "TUI poller did not dequeue the inject event" + drain_thread.start() + + # The active loop must wait for the poller's atomic route/requeue section; + # returning here would miss a ready same-turn result because the queue is + # temporarily empty. + assert not drain_done.wait(0.5) + + release_dequeue.set() + poller_thread.join(3) + drain_thread.join(3) + + assert not poller_thread.is_alive() + assert not drain_thread.is_alive() + assert drained == {"count": 1} + assert messages[-1]["role"] == "user" + assert messages[-1]["content"] == "ready" + assert len(claims) == 1 + assert claims[0][0] == "task:0" + assert claims[0][1].startswith("conversation-loop:") + assert isolated_queue.empty() + finally: + release_dequeue.set() + stop_poller.set() + poller_thread.join(3) + drain_thread.join(3) + server._sessions.pop(sid, None) + while not isolated_queue.empty(): + isolated_queue.get_nowait() + + +def test_tui_claim_loss_does_not_leave_session_busy(monkeypatch): + """A competing durable consumer cannot strand the TUI in running state.""" + import queue as _queue_mod + + import tools.async_delegation as delegation_mod + import tools.process_registry as registry_mod + from tools.process_registry import process_registry + + stop_poller = threading.Event() + + class _StopOnGetQueue(_queue_mod.Queue): + def get(self, block=True, timeout=None): + item = super().get(block=block, timeout=timeout) + stop_poller.set() + return item + + isolated_queue: _queue_mod.Queue = _StopOnGetQueue() + monkeypatch.setattr(process_registry, "completion_queue", isolated_queue) + monkeypatch.setattr(registry_mod, "format_process_notification", lambda _evt: "ready") + monkeypatch.setattr(delegation_mod, "claim_event_delivery", lambda *_args: None) + monkeypatch.setattr(server, "_emit", lambda *_args, **_kwargs: None) + + sid = "sid-tui-claim-loss" + sess = _session(running=False, session_key="session-tui-claim-loss") + event = { + "type": "async_delegation", + "delegation_id": "deleg-tui-claim-loss", + "delivery_event_key": "task:0", + "result_delivery": "after_turn", + "origin_ui_session_id": sid, + "session_key": "session-tui-claim-loss", + } + isolated_queue.put(event) + server._sessions[sid] = sess + + try: + server._notification_poller_loop(stop_poller, sid, sess) + + assert sess["running"] is False + assert isolated_queue.empty() + finally: + server._sessions.pop(sid, None) + while not isolated_queue.empty(): + isolated_queue.get_nowait() + + def test_session_save_writes_under_hermes_home_with_system_prompt(monkeypatch, tmp_path): """TUI /save (session.save RPC) must snapshot under the Hermes profile home β€” not the project/workspace CWD β€” and include the system prompt, diff --git a/tools/process_registry.py b/tools/process_registry.py index 411f26c9be12..bd73ebc735a2 100644 --- a/tools/process_registry.py +++ b/tools/process_registry.py @@ -171,6 +171,13 @@ def __init__(self): # gateway drain this after each agent turn to auto-trigger new turns. import queue as _queue_mod self.completion_queue: _queue_mod.Queue = _queue_mod.Queue() + # Queue.get() removes an event before a consumer has decided whether to + # claim, drop, or requeue it. The TUI poller and an active conversation + # loop can otherwise race in that temporary-empty window, making a ready + # result_delivery=inject event miss its same-turn boundary. Consumers + # that route/claim completion events hold this lock only for the bounded + # dequeue -> decision handoff, never while an agent/model turn runs. + self.completion_routing_lock = threading.RLock() # Rehydrate durable delegation completions only at registry startup. # Consumers still inject them as fresh turns through this existing rail. try: diff --git a/tui_gateway/server.py b/tui_gateway/server.py index ca5876ac00be..043673085920 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -8669,81 +8669,110 @@ def _notification_poller_loop( ) with session["history_lock"]: session["running"] = False - try: - evt = process_registry.completion_queue.get(timeout=0.5) - except Exception: + # Route one completion atomically with the active conversation-loop + # drain. Queue.get() removes the event before we know whether this TUI + # owns it or must requeue it; without the shared routing lock, the + # conversation loop can observe a temporary-empty queue and miss a ready + # same-turn inject at its safe boundary. + _route_action = "empty" + evt = None + text = None + _claim = None + with process_registry.completion_routing_lock: + try: + evt = process_registry.completion_queue.get_nowait() + except Exception: + evt = None + + if evt is not None: + # Multiple desktop sessions share this one process-wide queue. + # Leave foreign events for their proven owner. + if _notification_event_belongs_elsewhere(sid, session, evt): + process_registry.completion_queue.put(evt) + _route_action = "requeued_foreign" + else: + # Addressed events require positive ownership proof. Truly + # ownerless ordinary notifications retain legacy global + # delivery. + requires_owner = _notification_event_requires_owner(evt) + if requires_owner and not _session_owns_notification_event( + sid, session, evt + ): + log = ( + logger.warning + if evt.get("type") == "async_delegation" + else logger.debug + ) + log( + "Dropping unowned %s notification (origin=%r key=%r) " + "instead of delivering to session %s", + evt.get("type", "completion"), + str(evt.get("origin_ui_session_id") or ""), + str(evt.get("session_key") or ""), + sid, + ) + _route_action = "dropped" + else: + _evt_sid = evt.get("session_id", "") + if ( + evt.get("type") == "completion" + and process_registry.is_completion_consumed(_evt_sid) + ): + _route_action = "dropped" + else: + text = format_process_notification(evt) + if not text: + _route_action = "dropped" + else: + # Only emit the same notification identity once; + # busy-session requeues otherwise surface the same + # status every poll tick. + _dedup_key = _notification_event_dedup_key(evt) + if _dedup_key not in _emitted: + _emit( + "status.update", + sid, + {"kind": "process", "text": text}, + ) + _emitted.add(_dedup_key) + + with session["history_lock"]: + if session.get("running"): + process_registry.completion_queue.put(evt) + _route_action = "requeued_busy" + else: + from tools.async_delegation import ( + claim_event_delivery, + ) + + _claim = claim_event_delivery(evt, "tui-poller") + if _claim is None: + # The active loop or another durable + # consumer won the event. Do not mark + # this TUI session busy for work it + # will never dispatch. + _route_action = "claimed_elsewhere" + else: + session["running"] = True + _route_action = "dispatch" + + if _route_action == "empty": + time.sleep(0.5) continue - - # Multiple desktop sessions share this one process-wide queue. Only - # consume events that belong to *this* session β€” otherwise a background - # process started in session A would surface its completion in whichever - # session's poller happened to wake first (Ben's "reported in a - # different session" bug). Leave foreign events for their owner. - if _notification_event_belongs_elsewhere(sid, session, evt): - process_registry.completion_queue.put(evt) + if _route_action == "requeued_foreign": time.sleep(0.1) continue - - # What reaches here is not owned by another LIVE session. Addressed - # events still require positive proof before injection: exact UI origin, - # direct durable key, or compression lineage. If none proves ownership, - # the event is orphaned and must not be adopted by this chat. Truly - # ownerless ordinary notifications retain legacy global delivery. - requires_owner = _notification_event_requires_owner(evt) - if requires_owner and not _session_owns_notification_event(sid, session, evt): - log = ( - logger.warning - if evt.get("type") == "async_delegation" - else logger.debug - ) - log( - "Dropping unowned %s notification (origin=%r key=%r) instead " - "of delivering to session %s", - evt.get("type", "completion"), - str(evt.get("origin_ui_session_id") or ""), - str(evt.get("session_key") or ""), - sid, - ) - continue - - _evt_sid = evt.get("session_id", "") - if evt.get("type") == "completion" and process_registry.is_completion_consumed(_evt_sid): - continue - - text = format_process_notification(evt) - if not text: - continue - - # Only emit the same notification identity to TUI once β€” re-queued - # completions get re-emitted every 0.5s otherwise when session is busy, - # while distinct watch_match events from the same process must remain - # visible independently. - _dedup_key = _notification_event_dedup_key(evt) - if _dedup_key not in _emitted: - _emit("status.update", sid, {"kind": "process", "text": text}) - _emitted.add(_dedup_key) - - _requeued = False - with session["history_lock"]: - if session.get("running"): - process_registry.completion_queue.put(evt) - _requeued = True - else: - session["running"] = True - if _requeued: - # Back off before re-polling: the re-queued event keeps the queue - # non-empty, so without a sleep this loop spins at full speed - # (100% CPU, GIL churn) for as long as the session stays busy. + if _route_action == "requeued_busy": + # The queue stays non-empty while the session is busy. Back off + # outside the routing lock so the active loop can claim the event. time.sleep(0.25) continue + if _route_action != "dispatch": + continue + assert evt is not None and text is not None and _claim is not None rid = f"__notif__{int(time.time() * 1000)}" - from tools.async_delegation import ( - claim_event_delivery, complete_event_delivery, release_event_delivery, - ) - _claim = claim_event_delivery(evt, "tui-poller") - if _claim is None: - continue + from tools.async_delegation import complete_event_delivery, release_event_delivery try: _emit("message.start", sid) if evt.get("type") == "async_delegation": @@ -8768,60 +8797,82 @@ def _notification_poller_loop( with session["history_lock"]: session["running"] = False - # Drain any remaining events after stop signal (process all pending - # before exiting so nothing is lost on shutdown). Events owned by other - # live sessions are set aside and re-queued so their poller still sees them. - # Orphaned events (owner gone) are dropped β€” same guard as the main loop. - deferred: list = [] - while not process_registry.completion_queue.empty(): - try: - evt = process_registry.completion_queue.get_nowait() - except Exception: - break - if _notification_event_belongs_elsewhere(sid, session, evt): - deferred.append(evt) - continue - # Same positive-proof rule as the live loop. Preserve the existing - # shutdown behavior for orphaned delegation payloads by deferring them - # for a later resume; ordinary addressed orphans are dropped. - requires_owner = _notification_event_requires_owner(evt) - if requires_owner and not _session_owns_notification_event(sid, session, evt): - if evt.get("type") == "async_delegation": - deferred.append(evt) - else: - logger.debug( - "Dropping unowned %s notification during shutdown drain " - "(origin=%r key=%r)", - evt.get("type", "completion"), - str(evt.get("origin_ui_session_id") or ""), - str(evt.get("session_key") or ""), - ) - continue - _evt_sid = evt.get("session_id", "") - if evt.get("type") == "completion" and process_registry.is_completion_consumed(_evt_sid): - continue - text = format_process_notification(evt) - if not text: - continue - - _dedup_key = _notification_event_dedup_key(evt) - if _dedup_key not in _emitted: - _emit("status.update", sid, {"kind": "process", "text": text}) - _emitted.add(_dedup_key) + # Drain the bounded queue snapshot after stop so pending notifications are + # not lost. Route each dequeue atomically with an active conversation-loop + # drain: teardown can signal this poller while its agent turn is still + # unwinding. Foreign/orphaned delegation events are requeued immediately; + # the fixed snapshot bound prevents spinning on them. + try: + shutdown_scan_count = process_registry.completion_queue.qsize() + except Exception: + shutdown_scan_count = 0 + for _ in range(max(0, shutdown_scan_count)): + evt = None + text = None + _claim = None + _shutdown_action = "skip" + with process_registry.completion_routing_lock: + try: + evt = process_registry.completion_queue.get_nowait() + except Exception: + break - with session["history_lock"]: - if session.get("running"): + if _notification_event_belongs_elsewhere(sid, session, evt): process_registry.completion_queue.put(evt) - break - session["running"] = True + continue - rid = f"__notif__{int(time.time() * 1000)}" - from tools.async_delegation import ( - claim_event_delivery, complete_event_delivery, release_event_delivery, - ) - _claim = claim_event_delivery(evt, "tui-poller") - if _claim is None: + requires_owner = _notification_event_requires_owner(evt) + if requires_owner and not _session_owns_notification_event( + sid, session, evt + ): + if evt.get("type") == "async_delegation": + process_registry.completion_queue.put(evt) + else: + logger.debug( + "Dropping unowned %s notification during shutdown drain " + "(origin=%r key=%r)", + evt.get("type", "completion"), + str(evt.get("origin_ui_session_id") or ""), + str(evt.get("session_key") or ""), + ) + continue + + _evt_sid = evt.get("session_id", "") + if ( + evt.get("type") == "completion" + and process_registry.is_completion_consumed(_evt_sid) + ): + continue + text = format_process_notification(evt) + if not text: + continue + + _dedup_key = _notification_event_dedup_key(evt) + if _dedup_key not in _emitted: + _emit("status.update", sid, {"kind": "process", "text": text}) + _emitted.add(_dedup_key) + + with session["history_lock"]: + if session.get("running"): + process_registry.completion_queue.put(evt) + _shutdown_action = "busy" + else: + from tools.async_delegation import claim_event_delivery + + _claim = claim_event_delivery(evt, "tui-poller") + if _claim is not None: + session["running"] = True + _shutdown_action = "dispatch" + + if _shutdown_action == "busy": + break + if _shutdown_action != "dispatch": continue + + assert evt is not None and text is not None and _claim is not None + rid = f"__notif__{int(time.time() * 1000)}" + from tools.async_delegation import complete_event_delivery, release_event_delivery + try: _emit("message.start", sid) if evt.get("type") == "async_delegation": @@ -8846,10 +8897,6 @@ def _notification_poller_loop( with session["history_lock"]: session["running"] = False - # Hand any other sessions' events back to the shared queue. - for evt in deferred: - process_registry.completion_queue.put(evt) - def _async_delegation_display_metadata(evt: dict) -> dict: """Build display-only metadata before the completion event is formatted.""" From da3800460db365993c63cbd384a5630921253e9c Mon Sep 17 00:00:00 2001 From: Xipong <217837358+Xipong@users.noreply.github.com> Date: Fri, 31 Jul 2026 00:27:15 +0300 Subject: [PATCH 6/7] fix(delegation): deliver ready after-turn batch children --- gateway/run.py | 86 ++-- tests/agent/test_delegation_inject.py | 172 +++++++- tests/gateway/test_completion_delivery.py | 98 +++++ tests/test_tui_gateway_server.py | 85 ++++ .../test_delegate_apiserver_background.py | 91 +++++ tools/async_delegation.py | 381 +++++++++++++++--- tools/delegate_tool.py | 51 +-- tools/process_registry.py | 194 ++++++--- tui_gateway/server.py | 16 +- .../docs/user-guide/features/delegation.md | 18 +- 10 files changed, 1004 insertions(+), 188 deletions(-) diff --git a/gateway/run.py b/gateway/run.py index 10b0fbf36335..08f239248e4e 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -20934,8 +20934,12 @@ def _completion_delivery_identity(evt: dict) -> Optional[tuple[str, str, object] evt_type = str(evt.get("type") or "") if evt_type == "async_delegation": producer_id = str(evt.get("delegation_id") or "") - event_key = str(evt.get("delivery_event_key") or "") - return (evt_type, producer_id, event_key) if producer_id else None + raw_keys = evt.get("delivery_event_keys") + if isinstance(raw_keys, (list, tuple)): + event_identity: object = tuple(str(key) for key in raw_keys if key) + else: + event_identity = str(evt.get("delivery_event_key") or "") + return (evt_type, producer_id, event_identity) if producer_id else None if evt_type == "completion": producer_id = str(evt.get("session_id") or "") started_at = evt.get("started_at") @@ -21162,44 +21166,52 @@ async def _async_delegation_watcher(self, interval: float = 2.0) -> None: # so requeue anything that isn't ours. requeue = [] async_events = [] - while not _pr.completion_queue.empty(): - try: - evt = _pr.completion_queue.get_nowait() - except Exception: - break - if evt.get("type") == "async_delegation": - async_events.append(evt) - else: - requeue.append(evt) - for evt in requeue: - _pr.completion_queue.put(evt) + with _pr.completion_routing_lock: + while not _pr.completion_queue.empty(): + try: + evt = _pr.completion_queue.get_nowait() + except Exception: + break + if evt.get("type") == "async_delegation": + async_events.append(evt) + else: + requeue.append(evt) + for evt in requeue: + _pr.completion_queue.put(evt) + from tools.async_delegation import coalesce_ready_after_turn_events + + async_events = coalesce_ready_after_turn_events(async_events) for evt in async_events: self._enrich_async_delegation_routing(evt) - # Gateway busy-session deferral for 'inject' events: - # if the parent session is currently running another - # turn, leave the inject queued so the conversation - # loop's safe-boundary drain can pick it up when the - # turn finishes. This keeps inject from being claimed - # away from the active parent. 'after_turn' events are - # always delivered here (the legacy path). + # Busy-session routing: + # - inject for the matching active turn stays queued for the + # conversation loop's safe-boundary drain; + # - after_turn stays queued until that foreground turn ends. + # A requeued after-turn envelope is re-coalescible, so siblings + # that finish meanwhile join the same next-boundary delivery. _rd = str(evt.get("result_delivery") or "after_turn").strip().lower() - if _rd == "inject": - _route_key = str(evt.get("session_key") or "").strip() - _event_turn_id = str(evt.get("parent_turn_id") or "") - _running_parent = self._running_agents.get(_route_key) - if _running_parent is _AGENT_PENDING_SENTINEL: - _pr.completion_queue.put(evt) - continue - if ( - _running_parent is not None - and _event_turn_id - and str( - getattr(_running_parent, "_active_turn_id", "") or "" - ) - == _event_turn_id - ): - _pr.completion_queue.put(evt) - continue + _route_key = str(evt.get("session_key") or "").strip() + _event_turn_id = str(evt.get("parent_turn_id") or "") + _running_parent = getattr(self, "_running_agents", {}).get( + _route_key + ) + if _running_parent is _AGENT_PENDING_SENTINEL: + _pr.completion_queue.put(evt) + continue + if _rd == "after_turn" and _running_parent is not None: + _pr.completion_queue.put(evt) + continue + if ( + _rd == "inject" + and _running_parent is not None + and _event_turn_id + and str( + getattr(_running_parent, "_active_turn_id", "") or "" + ) + == _event_turn_id + ): + _pr.completion_queue.put(evt) + continue synth_text = _format_gateway_process_notification(evt) if not synth_text: continue diff --git a/tests/agent/test_delegation_inject.py b/tests/agent/test_delegation_inject.py index 63fbe329acf0..d9625c70cd27 100644 --- a/tests/agent/test_delegation_inject.py +++ b/tests/agent/test_delegation_inject.py @@ -414,7 +414,7 @@ def runner(): assert all(event.get("delivery_event_key") for event in restored) -def test_after_turn_remains_default_and_emits_only_combined_batch_event(): +def test_after_turn_remains_default_and_coalesces_all_ready_children(): gate = threading.Event() def runner(): @@ -430,11 +430,154 @@ def runner(): ) assert process_registry.completion_queue.empty() gate.set() - event = process_registry.completion_queue.get(timeout=3) + deadline = time.time() + 3 + while process_registry.completion_queue.qsize() < 2 and time.time() < deadline: + time.sleep(0.01) + drained = process_registry.drain_notifications( + owns_event=lambda _event: True, + skip_poll_observed=False, + ) + assert len(drained) == 1 + event, text = drained[0] assert event["delegation_id"] == dispatched["delegation_id"] assert event["result_delivery"] == "after_turn" - assert "delivery_event_key" not in event + assert event["delivery_event_keys"] == ["task:0", "task:1"] assert [r["summary"] for r in event["results"]] == ["A", "B"] + assert "RESULTS READY" in text + claim = ad.claim_event_delivery(event, "test-after-turn-default") + assert claim + assert ad.complete_event_delivery(event, claim) + + +def test_batch_finalization_enqueues_ready_set_under_routing_lock(monkeypatch): + delegation_id = _record(goals=("A", "B"), delivery="after_turn") + with ad._records_lock: + record = dict(ad._records[delegation_id]) + children = [_child(0, "A"), _child(1, "B")] + + class TrackingLock: + def __init__(self): + self.inner = threading.RLock() + self.depth = 0 + + def __enter__(self): + self.inner.acquire() + self.depth += 1 + return self + + def __exit__(self, *_exc): + self.depth -= 1 + self.inner.release() + + tracking_lock = TrackingLock() + + class GuardedQueue(__import__("queue").Queue): + def put(self, item, *args, **kwargs): + assert tracking_lock.depth > 0 + return super().put(item, *args, **kwargs) + + guarded_queue = GuardedQueue() + monkeypatch.setattr(process_registry, "completion_routing_lock", tracking_lock) + monkeypatch.setattr(process_registry, "completion_queue", guarded_queue) + combined = {"results": children, "total_duration_seconds": 1.0} + parent_event = { + **record, + "type": "async_delegation", + "status": "completed", + "is_batch": True, + "results": children, + "completed_at": time.time(), + } + + ad._persist_batch_child_finalization( + record, parent_event, combined, "completed" + ) + + queued = [guarded_queue.get_nowait(), guarded_queue.get_nowait()] + assert [event["delivery_event_key"] for event in queued] == ["task:0", "task:1"] + + +def test_after_turn_coalesced_claim_is_atomic_across_ready_children(): + delegation_id = _record(goals=("A", "B"), delivery="after_turn") + assert ad.publish_batch_child_completion(delegation_id, 0, _child(0, "A")) + assert ad.publish_batch_child_completion(delegation_id, 1, _child(1, "B")) + children = [ + process_registry.completion_queue.get_nowait(), + process_registry.completion_queue.get_nowait(), + ] + grouped = ad.coalesce_ready_after_turn_events(children)[0] + + competing_claim = ad.claim_event_delivery(children[1], "competing-consumer") + assert competing_claim + assert ad.claim_event_delivery(grouped, "group-consumer") is None + # task:0's attempted group claim rolled back with the task:1 conflict. + assert _event_state(delegation_id, "task:0") == ("pending", 0) + assert _event_state(delegation_id, "task:1") == ("pending", 1) + + assert ad.release_event_delivery(children[1], competing_claim) + group_claim = ad.claim_event_delivery(grouped, "group-consumer") + assert group_claim + assert ad.complete_event_delivery(grouped, group_claim) + assert _event_state(delegation_id, "task:0") == ("delivered", 1) + assert _event_state(delegation_id, "task:1") == ("delivered", 2) + + +def test_group_release_prunes_attempt_capped_child_without_blocking_sibling(): + delegation_id = _record(goals=("fresh", "near-cap"), delivery="after_turn") + for index, summary in enumerate(("fresh", "near-cap")): + assert ad.publish_batch_child_completion( + delegation_id, index, _child(index, summary) + ) + children = [ + process_registry.completion_queue.get_nowait(), + process_registry.completion_queue.get_nowait(), + ] + near_cap = children[1] + for _ in range(ad._MAX_DELIVERY_ATTEMPTS - 1): + claim = ad.claim_event_delivery(near_cap, "failing-consumer") + assert claim + assert ad.release_event_delivery(near_cap, claim) + + grouped = ad.coalesce_ready_after_turn_events(children)[0] + group_claim = ad.claim_event_delivery(grouped, "group-consumer") + assert group_claim + assert ad.release_event_delivery(grouped, group_claim) + + assert grouped["delivery_event_keys"] == ["task:0"] + assert [result["summary"] for result in grouped["results"]] == ["fresh"] + assert _event_state(delegation_id, "task:0") == ("pending", 1) + assert _event_state(delegation_id, "task:1") == ( + "dropped", + ad._MAX_DELIVERY_ATTEMPTS, + ) + retry_claim = ad.claim_event_delivery(grouped, "retry-consumer") + assert retry_claim + assert ad.complete_event_delivery(grouped, retry_claim) + assert _event_state(delegation_id, "task:0") == ("delivered", 2) + + +def test_requeued_after_turn_envelope_absorbs_newly_ready_sibling(): + delegation_id = _record(goals=("A", "B", "C"), delivery="after_turn") + for index, summary in enumerate(("A", "B", "C")): + assert ad.publish_batch_child_completion( + delegation_id, index, _child(index, summary) + ) + children = [ + process_registry.completion_queue.get_nowait(), + process_registry.completion_queue.get_nowait(), + process_registry.completion_queue.get_nowait(), + ] + requeued_envelope = ad.coalesce_ready_after_turn_events(children[:2])[0] + process_registry.completion_queue.put(children[2]) + + merged = process_registry.collect_ready_after_turn_siblings(requeued_envelope) + + assert merged["delivery_event_keys"] == ["task:0", "task:1", "task:2"] + assert [result["summary"] for result in merged["results"]] == ["A", "B", "C"] + assert process_registry.completion_queue.empty() + claim = ad.claim_event_delivery(merged, "requeued-group-consumer") + assert claim + assert ad.complete_event_delivery(merged, claim) def test_inject_batch_finalization_rolls_back_children_if_parent_update_fails( @@ -456,7 +599,7 @@ def crash_before_parent_update(*_args, **_kwargs): monkeypatch.setattr(ad, "_update_completion_row", crash_before_parent_update) with pytest.raises(RuntimeError, match="simulated crash"): - ad._persist_inject_batch_finalization( + ad._persist_batch_child_finalization( event_record, parent_event, combined, "completed" ) @@ -464,8 +607,9 @@ def crash_before_parent_update(*_args, **_kwargs): assert _parent_state(delegation_id) == ("running", "pending") -def test_recovery_with_complete_inject_children_suppresses_parent_aggregate(): - delegation_id = _record(goals=("A", "B")) +@pytest.mark.parametrize("delivery", ["inject", "after_turn"]) +def test_recovery_with_complete_children_suppresses_parent_aggregate(delivery): + delegation_id = _record(goals=("A", "B"), delivery=delivery) assert ad.publish_batch_child_completion(delegation_id, 0, _child(0, "A")) assert ad.publish_batch_child_completion(delegation_id, 1, _child(1, "B")) while not process_registry.completion_queue.empty(): @@ -489,8 +633,9 @@ def test_recovery_with_complete_inject_children_suppresses_parent_aggregate(): } -def test_recovery_with_partial_inject_children_emits_only_terminal_gap_event(): - delegation_id = _record(goals=("A", "B")) +@pytest.mark.parametrize("delivery", ["inject", "after_turn"]) +def test_recovery_with_partial_children_emits_only_terminal_gap_event(delivery): + delegation_id = _record(goals=("A", "B"), delivery=delivery) assert ad.publish_batch_child_completion(delegation_id, 0, _child(0, "A")) process_registry.completion_queue.get_nowait() with ad._DB_LOCK, ad._transaction() as conn: @@ -511,12 +656,17 @@ def test_recovery_with_partial_inject_children_emits_only_terminal_gap_event(): "task:0", "terminal", } - terminal = next(event for event in events if event["delivery_event_key"] == "terminal") + terminal = next( + event for event in events if event["delivery_event_key"] == "terminal" + ) + assert terminal["status"] == "unknown" + assert terminal["result_delivery"] == delivery assert "1/2 batch child results" in terminal["error"] -def test_recovery_requires_exact_expected_batch_child_keys(): - delegation_id = _record(goals=("A", "B")) +@pytest.mark.parametrize("delivery", ["inject", "after_turn"]) +def test_recovery_requires_exact_expected_batch_child_keys(delivery): + delegation_id = _record(goals=("A", "B"), delivery=delivery) assert ad.publish_batch_child_completion(delegation_id, 0, _child(0, "A")) process_registry.completion_queue.get_nowait() with ad._records_lock: diff --git a/tests/gateway/test_completion_delivery.py b/tests/gateway/test_completion_delivery.py index 3c3e09967f41..ff8323736dea 100644 --- a/tests/gateway/test_completion_delivery.py +++ b/tests/gateway/test_completion_delivery.py @@ -112,6 +112,104 @@ def test_duplicate_async_queue_replay_injects_once(monkeypatch, isolated_registr adapter.handle_message.assert_awaited_once() +def test_gateway_watcher_coalesces_ready_after_turn_batch_children( + monkeypatch, isolated_registry, +): + isolated = queue.Queue() + monkeypatch.setattr(isolated_registry, "completion_queue", isolated) + base = { + **_async_event("deleg_after_turn_group"), + "result_delivery": "after_turn", + "is_batch": True, + "batch_size": 3, + "goals": ["zero", "one", "two"], + } + for index in (0, 1): + isolated.put( + { + **base, + "delivery_event_key": f"task:{index}", + "task_index": index, + "results": [ + { + "task_index": index, + "status": "completed", + "summary": f"ready-{index}", + } + ], + } + ) + + adapter = SimpleNamespace(handle_message=AsyncMock()) + runner = _runner(adapter) + deliver = AsyncMock(return_value=True) + runner._deliver_completion_notification = deliver + _stop_after_sleeps(monkeypatch, runner, count=2) + + asyncio.run(runner._async_delegation_watcher(interval=0)) + + deliver.assert_awaited_once() + text, grouped = deliver.await_args_list[0].args + assert grouped["delivery_event_keys"] == ["task:0", "task:1"] + assert [result["task_index"] for result in grouped["results"]] == [0, 1] + assert "RESULTS READY" in text and "2/3" in text + assert isolated.empty() + + +def test_gateway_after_turn_waits_for_idle_boundary_then_delivers_ready_group( + monkeypatch, isolated_registry, +): + isolated = queue.Queue() + monkeypatch.setattr(isolated_registry, "completion_queue", isolated) + base = { + **_async_event("deleg_after_turn_busy"), + "result_delivery": "after_turn", + "is_batch": True, + "batch_size": 3, + "goals": ["zero", "one", "two"], + } + for index in (0, 1): + isolated.put( + { + **base, + "delivery_event_key": f"task:{index}", + "task_index": index, + "results": [ + { + "task_index": index, + "status": "completed", + "summary": f"ready-{index}", + } + ], + } + ) + + adapter = SimpleNamespace(handle_message=AsyncMock()) + runner = _runner(adapter) + deliver = AsyncMock(return_value=True) + runner._deliver_completion_notification = deliver + runner._running_agents = {base["session_key"]: SimpleNamespace()} + _stop_after_sleeps(monkeypatch, runner, count=2) + + asyncio.run(runner._async_delegation_watcher(interval=0)) + + deliver.assert_not_awaited() + assert isolated.qsize() == 1 + queued_group = isolated.get_nowait() + assert queued_group["delivery_event_keys"] == ["task:0", "task:1"] + isolated.put(queued_group) + + runner._running = True + runner._running_agents = {} + _stop_after_sleeps(monkeypatch, runner, count=2) + asyncio.run(runner._async_delegation_watcher(interval=0)) + + deliver.assert_awaited_once() + _text, delivered_group = deliver.await_args_list[0].args + assert delivered_group["delivery_event_keys"] == ["task:0", "task:1"] + assert isolated.empty() + + def test_unroutable_async_event_is_not_requeued_forever( monkeypatch, isolated_registry, ): diff --git a/tests/test_tui_gateway_server.py b/tests/test_tui_gateway_server.py index 1e4d713f212b..9774fb3e6a1c 100644 --- a/tests/test_tui_gateway_server.py +++ b/tests/test_tui_gateway_server.py @@ -13271,6 +13271,91 @@ def get(self, block=True, timeout=None): isolated_queue.get_nowait() +def test_tui_after_turn_coalesces_all_ready_batch_children(monkeypatch): + """The idle TUI starts one synthetic turn for the ready child set.""" + import queue as _queue_mod + + import tools.async_delegation as delegation_mod + from tools.process_registry import process_registry + + stop_poller = threading.Event() + isolated_queue: _queue_mod.Queue = _queue_mod.Queue() + monkeypatch.setattr(process_registry, "completion_queue", isolated_queue) + turns = [] + claims = [] + completions = [] + + def claim(event, consumer): + claims.append((event, consumer)) + return "group-claim" + + def complete(event, claim_id): + completions.append((event, claim_id)) + return True + + def run_prompt(_rid, _sid, current_session, text, **_kwargs): + turns.append(text) + with current_session["history_lock"]: + current_session["running"] = False + stop_poller.set() + + monkeypatch.setattr(delegation_mod, "claim_event_delivery", claim) + monkeypatch.setattr(delegation_mod, "complete_event_delivery", complete) + monkeypatch.setattr(server, "_run_prompt_submit", run_prompt) + monkeypatch.setattr(server, "_emit", lambda *_args, **_kwargs: None) + + sid = "sid-tui-after-turn-group" + session_key = "session-tui-after-turn-group" + sess = _session(running=False, session_key=session_key) + base = { + "type": "async_delegation", + "delegation_id": "deleg-tui-after-turn-group", + "result_delivery": "after_turn", + "origin_ui_session_id": sid, + "session_key": session_key, + "is_batch": True, + "batch_size": 3, + "goals": ["zero", "one", "two"], + "role": "leaf", + "model": "m", + } + for index in (0, 1): + isolated_queue.put( + { + **base, + "delivery_event_key": f"task:{index}", + "task_index": index, + "results": [ + { + "task_index": index, + "status": "completed", + "summary": f"ready-{index}", + } + ], + } + ) + server._sessions[sid] = sess + + try: + server._notification_poller_loop(stop_poller, sid, sess) + + assert len(turns) == 1 + assert "RESULTS READY" in turns[0] + assert "2/3" in turns[0] + assert "ready-0" in turns[0] and "ready-1" in turns[0] + assert len(claims) == 1 + grouped = claims[0][0] + assert grouped["delivery_event_keys"] == ["task:0", "task:1"] + assert claims[0][1] == "tui-poller" + assert completions == [(grouped, "group-claim")] + assert isolated_queue.empty() + finally: + stop_poller.set() + server._sessions.pop(sid, None) + while not isolated_queue.empty(): + isolated_queue.get_nowait() + + def test_session_save_writes_under_hermes_home_with_system_prompt(monkeypatch, tmp_path): """TUI /save (session.save RPC) must snapshot under the Hermes profile home β€” not the project/workspace CWD β€” and include the system prompt, diff --git a/tests/tools/test_delegate_apiserver_background.py b/tests/tools/test_delegate_apiserver_background.py index e5fc3d8d3bc7..84630927b2ef 100644 --- a/tests/tools/test_delegate_apiserver_background.py +++ b/tests/tools/test_delegate_apiserver_background.py @@ -170,6 +170,97 @@ def staggered_child(task_index, goal, child=None, parent_agent=None, **kw): assert ad.active_count() == 0 +def test_after_turn_batch_delivers_ready_children_without_waiting_and_coalesces(monkeypatch): + """Ready siblings share one turn; an unfinished sibling cannot block them.""" + dt = _patch_delegate(monkeypatch) + threading = __import__("threading") + slow_gate = threading.Event() + first_pair_ready = threading.Event() + ready_lock = threading.Lock() + ready_indices = set() + + def staggered_child(task_index, goal, child=None, parent_agent=None, **kw): + if task_index == 2: + slow_gate.wait(timeout=5) + with ready_lock: + ready_indices.add(task_index) + if {0, 1}.issubset(ready_indices): + first_pair_ready.set() + return { + "task_index": task_index, + "status": "completed", + "summary": f"done: {goal}", + "api_calls": 1, + "duration_seconds": 0.1, + "model": "m", + "exit_reason": "completed", + } + + monkeypatch.setattr(dt, "_run_single_child", staggered_child) + set_session_vars( + platform="telegram", + chat_id="7", + session_key="agent:main:telegram:dm:7", + session_id="parent-sess", + async_delivery=True, + ) + parsed = json.loads( + dt.delegate_task( + tasks=[{"goal": "zero"}, {"goal": "one"}, {"goal": "two"}], + background=True, + result_delivery="after_turn", + parent_agent=_fake_parent(), + ) + ) + assert parsed["status"] == "dispatched", parsed + assert first_pair_ready.wait(timeout=2) + publish_deadline = time.time() + 2 + while process_registry.completion_queue.qsize() < 2 and time.time() < publish_deadline: + time.sleep(0.01) + assert process_registry.completion_queue.qsize() >= 2 + + drained = process_registry.drain_notifications( + owns_event=lambda _event: True, + skip_poll_observed=False, + ) + assert len(drained) == 1 + first_event, first_text = drained[0] + assert first_event["delivery_event_keys"] == ["task:0", "task:1"] + assert [result["task_index"] for result in first_event["results"]] == [0, 1] + assert "done: zero" in first_text + assert "done: one" in first_text + assert "done: two" not in first_text + + import tools.async_delegation as ad + + claim = ad.claim_event_delivery(first_event, "test-after-turn") + assert claim + assert ad.complete_event_delivery(first_event, claim) + + slow_gate.set() + deadline = time.time() + 2 + late = [] + while time.time() < deadline and not late: + late = process_registry.drain_notifications( + owns_event=lambda _event: True, + skip_poll_observed=False, + ) + if not late: + time.sleep(0.02) + assert len(late) == 1 + late_event, late_text = late[0] + assert late_event["delivery_event_keys"] == ["task:2"] + assert [result["task_index"] for result in late_event["results"]] == [2] + assert "done: two" in late_text + claim = ad.claim_event_delivery(late_event, "test-after-turn") + assert claim + assert ad.complete_event_delivery(late_event, claim) + finalize_deadline = time.time() + 2 + while ad.active_count() and time.time() < finalize_deadline: + time.sleep(0.01) + assert ad.active_count() == 0 + + def test_apiserver_session_with_id_dispatches_background(monkeypatch): """async_delivery=False + a raw session id (HERMES_SESSION_ID) β†’ background dispatch (the completion wakes the session via the diff --git a/tools/async_delegation.py b/tools/async_delegation.py index 99ebcc8e2884..5075a4b65a06 100644 --- a/tools/async_delegation.py +++ b/tools/async_delegation.py @@ -405,7 +405,9 @@ def _build_batch_child_event( ), "dispatched_at": record.get("dispatched_at"), "completed_at": completed_at, - "result_delivery": "inject", + "result_delivery": str( + record.get("result_delivery") or "after_turn" + ).strip().lower(), } @@ -431,15 +433,22 @@ def publish_batch_child_completion( task_index: int, result: Dict[str, Any], ) -> bool: - """Durably enqueue one ready child from an ``inject`` batch. + """Durably enqueue one ready child from an asynchronous batch. - The parent batch remains the execution/stall unit. This function only + The parent batch remains the execution/stall unit. This function only creates an independently claimable delivery event, keyed by task index. - Repeated publication is idempotent and never resets a delivered claim. + ``result_delivery`` changes which consumer may claim it, not when the child + row becomes durable. Repeated publication is idempotent and never resets a + delivered claim. """ with _records_lock: record = dict(_records.get(delegation_id) or {}) - if str(record.get("result_delivery") or "after_turn").lower() != "inject": + if not record or not bool(record.get("is_batch")): + return False + if str(record.get("result_delivery") or "after_turn").lower() not in { + "inject", + "after_turn", + }: return False event = _build_batch_child_event(record, task_index, result) @@ -454,6 +463,94 @@ def publish_batch_child_completion( return True +def _event_delivery_keys(event: Dict[str, Any]) -> list[str]: + raw_keys = event.get("delivery_event_keys") + if isinstance(raw_keys, (list, tuple)): + return list(dict.fromkeys(str(key) for key in raw_keys if key)) + key = str(event.get("delivery_event_key") or "") + return [key] if key else [] + + +def coalesce_ready_after_turn_events( + events: List[Dict[str, Any]], +) -> List[Dict[str, Any]]: + """Combine ready child rows from each after-turn batch at one boundary. + + SQLite and the shared queue retain one durable row per child. This helper + creates only a transient delivery envelope so every child remains + independently recoverable while one consumer can claim/ack the currently + ready set atomically. Envelopes are deliberately re-coalescible: a busy + consumer may requeue one while more siblings finish before its next boundary. + """ + + output: List[Dict[str, Any]] = [] + groups: Dict[str, Dict[str, Any]] = {} + seen_keys: Dict[str, set[str]] = {} + for event in events: + delivery = str(event.get("result_delivery") or "after_turn").strip().lower() + event_keys = _event_delivery_keys(event) + delegation_id = str(event.get("delegation_id") or "") + coalescible = ( + event.get("type") == "async_delegation" + and delivery == "after_turn" + and bool(event.get("is_batch")) + and bool(event_keys) + and all(key.startswith("task:") for key in event_keys) + and bool(delegation_id) + ) + if not coalescible: + output.append(event) + continue + + grouped = groups.get(delegation_id) + if grouped is None: + grouped = dict(event) + grouped.pop("delivery_event_key", None) + grouped["delivery_event_keys"] = [] + grouped["task_indices"] = [] + grouped["results"] = [] + grouped["live_transcripts"] = [] + grouped["coalesced_after_turn"] = True + groups[delegation_id] = grouped + seen_keys[delegation_id] = set() + output.append(grouped) + + new_keys = [ + key for key in event_keys if key not in seen_keys[delegation_id] + ] + if not new_keys: + continue + seen_keys[delegation_id].update(new_keys) + grouped["delivery_event_keys"].extend(new_keys) + new_key_set = set(new_keys) + for result in event.get("results") or []: + if not isinstance(result, dict): + continue + task_index = int(result.get("task_index", 0)) + if f"task:{task_index}" not in new_key_set: + continue + grouped["task_indices"].append(task_index) + grouped["results"].append(result) + grouped["live_transcripts"].extend( + path for path in (event.get("live_transcripts") or []) if path + ) + grouped["completed_at"] = max( + float(grouped.get("completed_at") or 0), + float(event.get("completed_at") or 0), + ) + + for grouped in groups.values(): + grouped["delivery_event_keys"].sort( + key=lambda key: int(key.split(":", 1)[1]) + ) + grouped["task_indices"] = sorted(set(grouped["task_indices"])) + grouped["results"].sort(key=lambda result: int(result.get("task_index", 0))) + grouped["live_transcripts"] = list( + dict.fromkeys(grouped["live_transcripts"]) + ) or None + return output + + def _build_batch_terminal_event( event_record: Dict[str, Any], combined: Dict[str, Any], @@ -489,7 +586,9 @@ def _build_batch_terminal_event( "error": combined.get("error"), "dispatched_at": event_record.get("dispatched_at"), "completed_at": now, - "result_delivery": "inject", + "result_delivery": str( + event_record.get("result_delivery") or "after_turn" + ).strip().lower(), } for key in ( "stalled_after_quiet_seconds", @@ -505,7 +604,7 @@ def _build_batch_terminal_event( def _publish_batch_terminal_event( event_record: Dict[str, Any], combined: Dict[str, Any], status: str ) -> bool: - """Deliver a parent-level inject failure not represented by child events.""" + """Deliver a batch-level failure not represented by child events.""" event = _build_batch_terminal_event(event_record, combined, status) if event is None: return False @@ -520,7 +619,7 @@ def _publish_batch_terminal_event( return True -def _persist_inject_batch_finalization( +def _persist_batch_child_finalization( event_record: Dict[str, Any], parent_event: Dict[str, Any], combined: Dict[str, Any], @@ -562,8 +661,9 @@ def _persist_inject_batch_finalization( from tools.process_registry import process_registry - for queued_event in newly_inserted: - process_registry.completion_queue.put(queued_event) + with process_registry.completion_routing_lock: + for queued_event in newly_inserted: + process_registry.completion_queue.put(queued_event) def _note_delivery_attempt(delegation_id: str) -> None: @@ -604,16 +704,20 @@ def recover_abandoned_delegations() -> int: restored_delivery = str( result_delivery or task.get("result_delivery") or "after_turn" ).lower() - if bool(task.get("is_batch")) and restored_delivery == "inject": - # Inject batches deliver child-scoped rows, never an aggregate. - # A crash after one or more child inserts must preserve that wire - # shape instead of manufacturing a contradictory parent unknown. + child_rows = [] + if bool(task.get("is_batch")): child_rows = conn.execute( """SELECT event_key, event_json FROM async_delegation_events WHERE delegation_id=? AND event_key LIKE 'task:%' ORDER BY event_key""", (delegation_id,), ).fetchall() + if bool(task.get("is_batch")) and ( + restored_delivery == "inject" or bool(child_rows) + ): + # Child-scoped batches never recover through a contradictory + # aggregate. A crash after one or more child inserts must + # preserve the already-delivered/pending task identities. goals = list(task.get("goals") or []) expected_child_keys = [f"task:{index}" for index in range(len(goals))] child_results_by_key: dict[str, Dict[str, Any]] = {} @@ -659,6 +763,7 @@ def recover_abandoned_delegations() -> int: "role": task.get("role"), "model": task.get("model"), "dispatched_at": dispatched_at, + "result_delivery": restored_delivery, } if not complete_children: terminal_event = _build_batch_terminal_event( @@ -675,7 +780,7 @@ def recover_abandoned_delegations() -> int: "results": child_results, "error": recovery_error, "completed_at": now, - "result_delivery": "inject", + "result_delivery": restored_delivery, } _update_completion_row( conn, @@ -794,6 +899,40 @@ def claim_completion_delivery(delegation_id: str, claim_id: str) -> bool: return cur.rowcount == 1 +class _DeliveryGroupConflict(RuntimeError): + """Rollback a multi-child settlement when any row lost ownership.""" + + +def _claim_child_event_group( + delegation_id: str, event_keys: List[str], claim_id: str +) -> bool: + now = time.time() + try: + with _DB_LOCK, _transaction() as conn: + for event_key in event_keys: + cur = conn.execute( + """UPDATE async_delegation_events + SET delivery_claim=?, delivery_claimed_at=?, + delivery_attempts=delivery_attempts+1, updated_at=? + WHERE delegation_id=? AND event_key=? + AND delivery_state='pending' + AND (delivery_claim IS NULL OR delivery_claimed_at < ?)""", + ( + claim_id, + now, + now, + delegation_id, + event_key, + now - _DELIVERY_CLAIM_LEASE_SECONDS, + ), + ) + if cur.rowcount != 1: + raise _DeliveryGroupConflict(event_key) + except _DeliveryGroupConflict: + return False + return True + + def _claim_child_event(delegation_id: str, event_key: str, claim_id: str) -> bool: now = time.time() with _DB_LOCK, _transaction() as conn: @@ -824,9 +963,13 @@ def claim_event_delivery(evt: Dict[str, Any], consumer: str) -> Optional[str]: if not delegation_id: return "" claim_id = f"{consumer}:{__import__('os').getpid()}:{uuid.uuid4().hex}" - event_key = str(evt.get("delivery_event_key") or "") - if event_key: - claimed = _claim_child_event(delegation_id, event_key, claim_id) + event_keys = _event_delivery_keys(evt) + if "delivery_event_keys" in evt: + claimed = bool(event_keys) and _claim_child_event_group( + delegation_id, event_keys, claim_id + ) + elif event_keys: + claimed = _claim_child_event(delegation_id, event_keys[0], claim_id) else: claimed = claim_completion_delivery(delegation_id, claim_id) return claim_id if claimed else None @@ -840,8 +983,27 @@ def renew_event_delivery(evt: Dict[str, Any], claim_id: str) -> bool: delegation_id = str(evt.get("delegation_id") or "") if not delegation_id: return False - event_key = str(evt.get("delivery_event_key") or "") + event_keys = _event_delivery_keys(evt) now = time.time() + if "delivery_event_keys" in evt: + if not event_keys: + return False + try: + with _DB_LOCK, _transaction() as conn: + for event_key in event_keys: + cur = conn.execute( + """UPDATE async_delegation_events + SET delivery_claimed_at=?, updated_at=? + WHERE delegation_id=? AND event_key=? + AND delivery_state='pending' AND delivery_claim=?""", + (now, now, delegation_id, event_key, claim_id), + ) + if cur.rowcount != 1: + raise _DeliveryGroupConflict(event_key) + except _DeliveryGroupConflict: + return False + return True + event_key = event_keys[0] if event_keys else "" with _DB_LOCK, _transaction() as conn: if event_key: cur = conn.execute( @@ -870,8 +1032,22 @@ def get_event_delivery_state(evt: Dict[str, Any]) -> Optional[str]: delegation_id = str(evt.get("delegation_id") or "") if not delegation_id: return None - event_key = str(evt.get("delivery_event_key") or "") + event_keys = _event_delivery_keys(evt) with _DB_LOCK, _transaction() as conn: + if "delivery_event_keys" in evt: + if not event_keys: + return None + placeholders = ",".join("?" for _ in event_keys) + rows = conn.execute( + f"""SELECT delivery_state FROM async_delegation_events + WHERE delegation_id=? AND event_key IN ({placeholders})""", + (delegation_id, *event_keys), + ).fetchall() + if len(rows) != len(event_keys): + return None + states = {str(row[0]) for row in rows} + return states.pop() if len(states) == 1 else "mixed" + event_key = event_keys[0] if event_keys else "" if event_key: row = conn.execute( """SELECT delivery_state FROM async_delegation_events @@ -999,13 +1175,98 @@ def _release_child_event(delegation_id: str, event_key: str, claim_id: str) -> b return cur.rowcount == 1 +def _complete_child_event_group( + delegation_id: str, + event_keys: List[str], + claim_id: str, + state: str = "delivered", +) -> bool: + now = time.time() + try: + with _DB_LOCK, _transaction() as conn: + for event_key in event_keys: + cur = conn.execute( + """UPDATE async_delegation_events + SET delivery_state=?, delivered_at=?, updated_at=?, + delivery_claim=NULL, delivery_claimed_at=NULL + WHERE delegation_id=? AND event_key=? + AND delivery_state='pending' AND delivery_claim=?""", + (state, now, now, delegation_id, event_key, claim_id), + ) + if cur.rowcount != 1: + raise _DeliveryGroupConflict(event_key) + except _DeliveryGroupConflict: + return False + return True + + +def _release_child_event_group( + delegation_id: str, event_keys: List[str], claim_id: str +) -> tuple[bool, List[str]]: + now = time.time() + pending_keys: List[str] = [] + try: + with _DB_LOCK, _transaction() as conn: + for event_key in event_keys: + capped = conn.execute( + """UPDATE async_delegation_events + SET delivery_state='dropped', delivery_claim=NULL, + delivery_claimed_at=NULL, updated_at=? + WHERE delegation_id=? AND event_key=? + AND delivery_state='pending' AND delivery_claim=? + AND delivery_attempts>=?""", + ( + now, + delegation_id, + event_key, + claim_id, + _MAX_DELIVERY_ATTEMPTS, + ), + ) + if capped.rowcount == 1: + continue + released = conn.execute( + """UPDATE async_delegation_events + SET delivery_claim=NULL, delivery_claimed_at=NULL, updated_at=? + WHERE delegation_id=? AND event_key=? + AND delivery_state='pending' AND delivery_claim=?""", + (now, delegation_id, event_key, claim_id), + ) + if released.rowcount != 1: + raise _DeliveryGroupConflict(event_key) + pending_keys.append(event_key) + except _DeliveryGroupConflict: + return False, event_keys + return True, pending_keys + + +def _retain_group_event_keys(evt: Dict[str, Any], event_keys: List[str]) -> None: + """Keep only rows that remain pending after a grouped release.""" + + keep = set(event_keys) + evt["delivery_event_keys"] = list(event_keys) + evt["results"] = [ + result + for result in (evt.get("results") or []) + if isinstance(result, dict) + and f"task:{int(result.get('task_index', 0))}" in keep + ] + evt["task_indices"] = sorted( + int(result.get("task_index", 0)) for result in evt["results"] + ) + + def complete_event_delivery(evt: Dict[str, Any], claim_id: str) -> bool: if not claim_id or evt.get("type") != "async_delegation": return False delegation_id = str(evt.get("delegation_id") or "") - event_key = str(evt.get("delivery_event_key") or "") - if event_key: - completed = _complete_child_event(delegation_id, event_key, claim_id) + event_keys = _event_delivery_keys(evt) + if "delivery_event_keys" in evt: + completed = bool(event_keys) and _complete_child_event_group( + delegation_id, event_keys, claim_id + ) + elif event_keys: + completed = _complete_child_event(delegation_id, event_keys[0], claim_id) else: completed = complete_completion_delivery(delegation_id, claim_id) return completed or get_event_delivery_state(evt) == "delivered" @@ -1015,9 +1276,18 @@ def release_event_delivery(evt: Dict[str, Any], claim_id: str) -> bool: if not claim_id or evt.get("type") != "async_delegation": return False delegation_id = str(evt.get("delegation_id") or "") - event_key = str(evt.get("delivery_event_key") or "") - if event_key: - return _release_child_event(delegation_id, event_key, claim_id) + event_keys = _event_delivery_keys(evt) + if "delivery_event_keys" in evt: + if not event_keys: + return False + released, pending_keys = _release_child_event_group( + delegation_id, event_keys, claim_id + ) + if released: + _retain_group_event_keys(evt, pending_keys) + return released + if event_keys: + return _release_child_event(delegation_id, event_keys[0], claim_id) return release_completion_delivery(delegation_id, claim_id) @@ -1025,9 +1295,14 @@ def drop_event_delivery(evt: Dict[str, Any], claim_id: str) -> None: if not claim_id or evt.get("type") != "async_delegation": return delegation_id = str(evt.get("delegation_id") or "") - event_key = str(evt.get("delivery_event_key") or "") - if event_key: - _complete_child_event(delegation_id, event_key, claim_id, state="dropped") + event_keys = _event_delivery_keys(evt) + if "delivery_event_keys" in evt: + if event_keys: + _complete_child_event_group( + delegation_id, event_keys, claim_id, state="dropped" + ) + elif event_keys: + _complete_child_event(delegation_id, event_keys[0], claim_id, state="dropped") else: drop_completion_delivery(delegation_id, claim_id) @@ -1455,10 +1730,11 @@ def dispatch_async_delegation_batch( parallelism is bounded separately by ``max_concurrent_children``), so a single ``delegate_task`` fan-out never exhausts the async pool by itself. - ``after_turn`` publishes one consolidated completion after every child is - done. ``inject`` uses the same batch execution unit but each child publishes - an independently durable event as it becomes ready; finalization only - persists the aggregate result/status for observability and recovery. + Both delivery modes publish each child into an independently durable row as + it becomes ready. ``inject`` may claim rows at same-turn safe boundaries; + ``after_turn`` coalesces every ready row at the next available turn boundary. + Finalization only persists aggregate status for observability/recovery and + idempotently fills any child row missed by its completion callback. Returns ``{"status": "dispatched", "delegation_id": ...}`` on success or ``{"status": "rejected", "error": ...}`` when the async pool is at @@ -1565,7 +1841,7 @@ def _worker() -> None: def _finalize_batch( delegation_id: str, combined: Dict[str, Any], status: str ) -> None: - """Mark a batch record complete and push ONE combined completion event.""" + """Mark a batch complete and persist any missing child delivery events.""" claimed = _begin_finalization(delegation_id) if claimed is None: return @@ -1578,17 +1854,7 @@ def _finalize_batch( def _push_batch_completion_event( event_record: Dict[str, Any], combined: Dict[str, Any], status: str ) -> None: - """Push a combined async-delegation batch completion event.""" - try: - from tools.process_registry import process_registry - except Exception as exc: # pragma: no cover - logger.error( - "Async delegation batch %s finished but process_registry import " - "failed; result lost: %s", - event_record.get("delegation_id"), exc, - ) - return - + """Finalize one child-scoped async-delegation batch.""" dispatched_at = event_record.get("dispatched_at") or time.time() completed_at = event_record.get("completed_at") or time.time() evt = { @@ -1631,24 +1897,13 @@ def _push_batch_completion_event( ): if _k in combined: evt[_k] = combined[_k] - if str(event_record.get("result_delivery") or "after_turn").lower() == "inject": - # Child callbacks publish incrementally for same-turn visibility. The - # finalizer idempotently inserts any missed child and commits the parent - # terminal row in one SQLite transaction; only newly inserted events are - # queued after that transaction commits. - _persist_inject_batch_finalization( - event_record, evt, combined, status - ) - return - _persist_completion(evt, combined) - try: - process_registry.completion_queue.put(evt) - except Exception as exc: # pragma: no cover - logger.error( - "Async delegation batch %s: failed to enqueue completion event; " - "result lost: %s", - event_record.get("delegation_id"), exc, - ) + # Child callbacks publish incrementally in both delivery modes. The + # finalizer idempotently inserts any missed child and commits the parent + # terminal row in one SQLite transaction; only newly inserted events are + # queued after that transaction commits. Consumers decide whether ready + # children belong in the current turn (inject) or the next turn + # (after_turn). + _persist_batch_child_finalization(event_record, evt, combined, status) def _ensure_stale_monitor() -> None: diff --git a/tools/delegate_tool.py b/tools/delegate_tool.py index be1dd3081260..ece2c7a85222 100644 --- a/tools/delegate_tool.py +++ b/tools/delegate_tool.py @@ -2816,8 +2816,8 @@ def delegate_task( # Background delegation applies to both single tasks and batches. A top-level # call is one async execution/stall unit. result_delivery determines whether - # ready children surface independently at safe boundaries or the completed - # batch follows the legacy consolidated synthetic-turn path. + # ready children surface independently at same-turn safe boundaries or are + # coalesced with every ready sibling at the next available turn boundary. background = is_truthy_value(background, default=False) if background is not None else False # result_delivery controls how child results reach the parent: @@ -2990,8 +2990,8 @@ def delegate_task( children.append((i, t, child)) def _publish_ready_result(entry: Dict[str, Any]) -> None: - """Publish one inject child without waiting for its batch siblings.""" - if not background or _delivery != "inject": + """Publish one ready child without waiting for its batch siblings.""" + if not background: return task_index = int(entry.get("task_index", 0)) payload = dict(entry) @@ -3005,7 +3005,7 @@ def _publish_ready_result(entry: Dict[str, Any]) -> None: # _push_batch_completion_event republishes all children # idempotently as a durable safety net. logger.exception( - "Failed to publish ready inject child %s/%s", + "Failed to publish ready child %s/%s", live_deleg_id, task_index, ) @@ -3015,10 +3015,9 @@ def _execute_and_aggregate(*, honor_parent_interrupt: bool = True) -> dict: fire subagent_stop hooks + cost rollup, and return the combined result dict. Used by BOTH the synchronous path and the background runner. In the background case this whole function runs on the daemon executor, so - the parent turn isn't blocked β€” but the batch still JOINS on itself - here (all children must finish) before producing ONE consolidated - results block. That is the contract: fan-out runs in the background, - waits on each other, and returns together. + the parent turn isn't blocked. Each child is published independently as + it becomes ready; this final aggregate is bookkeeping plus an idempotent + safety net for any child callback that failed before persistence. """ if n_tasks == 1: # Single task -- run directly (no thread pool overhead) @@ -3194,8 +3193,9 @@ def _execute_and_aggregate(*, honor_parent_interrupt: bool = True) -> dict: # ----- Background dispatch: run the WHOLE batch as one async unit ----- # _execute_and_aggregate owns child execution and still returns one ordered - # aggregate. In inject mode its completion callback also publishes each - # child immediately; in after_turn mode only the aggregate is delivered. + # bookkeeping aggregate. Its completion callback publishes each child + # immediately in both delivery modes; consumers choose current-turn inject + # versus next-turn ready-set coalescing. if background: from tools.async_delegation import dispatch_async_delegation_batch from tools.approval import get_current_session_key @@ -3372,8 +3372,9 @@ def _batch_progress(): delegation_id=live_deleg_id, progress_fn=_batch_progress, # Delivery mode: 'inject' (synthetic at safe boundaries) or - # 'after_turn' (legacy completion_queue path). Default 'after_turn' - # preserves backward compatibility with old task_json entries. + # 'after_turn' (ready children coalesced at the next available turn + # boundary). Default 'after_turn' preserves compatibility with old + # task_json entries. result_delivery=_delivery, parent_turn_id=_parent_turn_id, ) @@ -3400,9 +3401,10 @@ def _batch_progress(): "continue." if n == 1 else f"{n} subagents are running in parallel in the background. You " - f"and the user can keep working; their consolidated results " - f"re-enter as a single message once ALL finish. Do not wait " - f"or poll β€” just continue." + f"and the user can keep working; every result ready at the next " + f"turn boundary re-enters in one grouped message without waiting " + f"for slower siblings. Later results arrive in later grouped " + f"messages. Do not wait or poll β€” just continue." ) payload = { "status": "dispatched", @@ -3763,8 +3765,9 @@ def _build_top_level_description() -> str: "each ready child is appended only after complete tool results and before " "the next model request, including one final reconciliation boundary. " "Choose result_delivery='after_turn' (the default) for independent work: " - "single results and consolidated batches arrive as separate synthetic " - "turns after the foreground turn. Neither mode waits for running children. " + "at each available turn boundary, every currently ready batch child is " + "grouped into one synthetic turn; slower siblings arrive in later grouped " + "turns. Neither mode waits for running children. " "Do NOT wait or poll; continue after dispatching.\n\n" "LIVE TRANSCRIPTS: the dispatch response includes 'live_transcripts' β€” " "one append-only human-readable log file per task (under " @@ -3957,9 +3960,9 @@ def _build_dynamic_schema_overrides() -> dict: "description": ( "DEPRECATED / IGNORED. Top-level single and batch " "delegations run in the background automatically β€” you do " - "not need to (and cannot) opt in or out. A single result or " - "consolidated batch result re-enters the conversation when " - "the work finishes; just continue working in the meantime. " + "not need to (and cannot) opt in or out. Ready results re-enter " + "the conversation according to result_delivery; just continue " + "working in the meantime. " "Setting this has no effect; the parameter remains only for " "backward compatibility." ), @@ -3978,8 +3981,10 @@ def _build_dynamic_schema_overrides() -> dict: "the bounded reconciliation window become separate late-result " "turns. " "'after_turn' (default): use for independent background work; " - "the result arrives in a separate synthetic turn after the " - "foreground turn. Running children are never waited on." + "at each available turn boundary, all currently ready children " + "are grouped into one synthetic turn, while slower siblings " + "arrive in later grouped turns. Running children are never " + "waited on." ), }, }, diff --git a/tools/process_registry.py b/tools/process_registry.py index bd73ebc735a2..c3c2402ad09d 100644 --- a/tools/process_registry.py +++ b/tools/process_registry.py @@ -1239,6 +1239,68 @@ def _drain_should_skip( skip_poll_observed and session_id in self._poll_observed ) + def collect_ready_after_turn_siblings(self, seed: dict) -> dict: + """Fold queued ready siblings into one transient after-turn envelope. + + The caller may already hold ``completion_routing_lock``; it is an RLock + so this helper is safe at both direct-consumer and shared-drain seams. + Only the bounded queue snapshot present at this delivery boundary is + inspected. Foreign delegations and later arrivals remain queued. + """ + + from tools.async_delegation import coalesce_ready_after_turn_events + + def delivery_keys(event: dict) -> "list[str]": + raw_keys = event.get("delivery_event_keys") + if isinstance(raw_keys, (list, tuple)): + return [str(key) for key in raw_keys if key] + key = str(event.get("delivery_event_key") or "") + return [key] if key else [] + + delivery = str(seed.get("result_delivery") or "after_turn").strip().lower() + seed_keys = delivery_keys(seed) + delegation_id = str(seed.get("delegation_id") or "") + if not ( + seed.get("type") == "async_delegation" + and delivery == "after_turn" + and bool(seed.get("is_batch")) + and bool(seed_keys) + and all(key.startswith("task:") for key in seed_keys) + and delegation_id + ): + return seed + + siblings = [seed] + requeue: "list[dict]" = [] + with self.completion_routing_lock: + try: + scan_count = self.completion_queue.qsize() + except Exception: + scan_count = 0 + for _ in range(max(0, scan_count)): + try: + candidate = self.completion_queue.get_nowait() + except Exception: + break + candidate_keys = delivery_keys(candidate) + if ( + candidate.get("type") == "async_delegation" + and str( + candidate.get("result_delivery") or "after_turn" + ).strip().lower() + == "after_turn" + and bool(candidate.get("is_batch")) + and str(candidate.get("delegation_id") or "") == delegation_id + and bool(candidate_keys) + and all(key.startswith("task:") for key in candidate_keys) + ): + siblings.append(candidate) + else: + requeue.append(candidate) + for candidate in requeue: + self.completion_queue.put(candidate) + return coalesce_ready_after_turn_events(siblings)[0] + def drain_notifications( self, session_key: str = "", @@ -1275,55 +1337,69 @@ def drain_notifications( filter is provided, ownerless async-delegation events remain fail-closed and require positive proof. """ - results: "list[tuple[dict, str]]" = [] + owned_events: "list[dict]" = [] requeue: "list[dict]" = [] - while not self.completion_queue.empty(): - try: - evt = self.completion_queue.get_nowait() - except Exception: - break - # Positive-proof ownership beats bare key equality. Delegation - # payloads always require proof; ordinary events require it once - # they carry routing metadata. Ownerless ordinary events preserve - # legacy single-session delivery. - is_async_delegation = evt.get("type") == "async_delegation" - evt_session_key = str(evt.get("session_key") or "") - evt_origin_sid = str(evt.get("origin_ui_session_id") or "") - requires_positive_proof = is_async_delegation or bool( - evt_session_key or evt_origin_sid - ) - if owns_event is not None and requires_positive_proof: + # One routing boundary owns the queue snapshot. This is the same lock + # used by the TUI poller and same-turn inject drain; it closes the + # temporary-empty race without spanning formatting or any model turn. + with self.completion_routing_lock: + while not self.completion_queue.empty(): try: - owned = bool(owns_event(evt)) + evt = self.completion_queue.get_nowait() except Exception: - owned = False # fail closed β€” never leak on a broken check - if not owned: + break + # Positive-proof ownership beats bare key equality. Delegation + # payloads always require proof; ordinary events require it once + # they carry routing metadata. Ownerless ordinary events preserve + # legacy single-session delivery. + is_async_delegation = evt.get("type") == "async_delegation" + evt_session_key = str(evt.get("session_key") or "") + evt_origin_sid = str(evt.get("origin_ui_session_id") or "") + requires_positive_proof = is_async_delegation or bool( + evt_session_key or evt_origin_sid + ) + if owns_event is not None and requires_positive_proof: + try: + owned = bool(owns_event(evt)) + except Exception: + owned = False # fail closed β€” never leak on a broken check + if not owned: + requeue.append(evt) + continue + elif session_key and requires_positive_proof: + if evt_session_key != session_key: + requeue.append(evt) + continue + elif is_async_delegation and evt.get("restored"): + # Durable restore can enqueue previous-process payloads into a + # fresh registry. An unfiltered legacy drain cannot prove + # ownership, so leave those events queued for the owner. requeue.append(evt) continue - elif session_key and requires_positive_proof: - if evt_session_key != session_key: - requeue.append(evt) + # Local consumed/observed state may suppress only events this + # session owns (or legacy ownerless ordinary events). Routing must + # happen first so a foreign session cannot drop the owner's event. + _evt_sid = evt.get("session_id", "") + if evt.get("type") == "completion" and self._drain_should_skip( + _evt_sid, skip_poll_observed=skip_poll_observed + ): continue - elif is_async_delegation and evt.get("restored"): - # Durable restore can enqueue previous-process payloads into a - # fresh registry. An unfiltered legacy drain cannot prove - # ownership, so leave those events queued for the owner. - requeue.append(evt) - continue - # Local consumed/observed state may suppress only events this - # session owns (or legacy ownerless ordinary events). Routing must - # happen first so a foreign session cannot drop the owner's event. - _evt_sid = evt.get("session_id", "") - if evt.get("type") == "completion" and self._drain_should_skip( - _evt_sid, skip_poll_observed=skip_poll_observed - ): - continue + owned_events.append(evt) + for evt in requeue: + self.completion_queue.put(evt) + + try: + from tools.async_delegation import coalesce_ready_after_turn_events + + owned_events = coalesce_ready_after_turn_events(owned_events) + except Exception: + logger.exception("Could not coalesce ready after-turn delegation events") + results: "list[tuple[dict, str]]" = [] + for evt in owned_events: text = format_process_notification(evt) if text: results.append((evt, text)) - for evt in requeue: - self.completion_queue.put(evt) return results def get(self, session_id: str) -> Optional[ProcessSession]: @@ -2133,15 +2209,22 @@ def _format_async_delegation(evt: dict) -> str: dispatched_at = evt.get("dispatched_at") completed_at = evt.get("completed_at") or _time.time() - # ----- Batch result: aggregate after_turn or per-child inject block ----- - # Aggregate events carry every result; inject child events carry one result - # plus its original batch index. Both use the same stable formatter. + # ----- Batch result: legacy aggregate or child-scoped delivery envelope ----- + # Durable child rows may arrive singly (inject) or be coalesced at an + # after-turn consumer boundary. Both use the same stable per-task renderer. batch_results = evt.get("results") if evt.get("is_batch") or isinstance(batch_results, list): results = batch_results or [] goals = evt.get("goals") or [] + raw_event_keys = evt.get("delivery_event_keys") + grouped_event_keys = ( + [str(key) for key in raw_event_keys if key] + if isinstance(raw_event_keys, (list, tuple)) + else [] + ) child_event = str(evt.get("delivery_event_key") or "").startswith("task:") - n = int(evt.get("batch_size") or 0) if child_event else 0 + child_scoped = child_event or bool(grouped_event_keys) + n = int(evt.get("batch_size") or 0) if child_scoped else 0 if n <= 0: n = len(results) if results else len(goals) total_dur = evt.get("total_duration_seconds", duration) @@ -2153,13 +2236,32 @@ def _format_async_delegation(evt: dict) -> str: "same-turn reconciliation; use it before continuing the current work.", "", ] + elif grouped_event_keys: + ready_count = len(results) + lines = [ + f"[ASYNC DELEGATION RESULTS READY β€” {deleg_id} β€” {ready_count}/{n}]", + ] + if ready_count >= n: + lines.append( + f"All {ready_count} background subagent results were ready at " + "this delivery boundary and are grouped below." + ) + else: + remaining = max(0, n - ready_count) + lines.append( + f"{ready_count} background subagent result(s) were ready at " + f"this delivery boundary and are grouped below. {remaining} " + "sibling(s) are still running; their results will arrive in a " + "later grouped delivery without blocking this one." + ) + lines.append("") else: lines = [ f"[ASYNC DELEGATION BATCH COMPLETE β€” {deleg_id}]", f"A background fan-out of {n} subagent(s) you dispatched earlier " - "has finished. All ran in parallel and waited on each other; their " - "consolidated results are below. You may have moved on since " - "dispatching β€” act on these or re-dispatch if things have changed.", + "has finished. Its consolidated results are below. You may have " + "moved on since dispatching β€” act on these or re-dispatch if things " + "have changed.", "", ] if isinstance(dispatched_at, (int, float)): diff --git a/tui_gateway/server.py b/tui_gateway/server.py index 043673085920..b57508d6eed5 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -8448,10 +8448,16 @@ def _notification_event_dedup_key(evt: dict) -> tuple: evt.get("suppressed", 0), ) if evt_type == "async_delegation": - # Async-delegation completions have no process session_id; without - # this the fallthrough keys every one as ("", "async_delegation") - # and the second completion's status update is suppressed forever. - return (evt.get("delegation_id", ""), evt_type) + # Child-scoped batches may deliver several ready rows together and the + # remaining rows later. Include the exact durable key set so a later + # portion of the same delegation is not visually suppressed. + raw_keys = evt.get("delivery_event_keys") + if isinstance(raw_keys, (list, tuple)): + event_keys = tuple(str(key) for key in raw_keys if key) + else: + event_key = str(evt.get("delivery_event_key") or "") + event_keys = (event_key,) if event_key else () + return (evt.get("delegation_id", ""), evt_type, event_keys) return (evt_sid, evt_type) @@ -8713,6 +8719,7 @@ def _notification_poller_loop( ) _route_action = "dropped" else: + evt = process_registry.collect_ready_after_turn_siblings(evt) _evt_sid = evt.get("session_id", "") if ( evt.get("type") == "completion" @@ -8837,6 +8844,7 @@ def _notification_poller_loop( ) continue + evt = process_registry.collect_ready_after_turn_siblings(evt) _evt_sid = evt.get("session_id", "") if ( evt.get("type") == "completion" diff --git a/website/docs/user-guide/features/delegation.md b/website/docs/user-guide/features/delegation.md index 80dc8152bf65..10f90d6680b7 100644 --- a/website/docs/user-guide/features/delegation.md +++ b/website/docs/user-guide/features/delegation.md @@ -37,9 +37,19 @@ delegate_task(tasks=[ running child. `result_delivery` controls only when an already-completed result is shown to the parent model: -- **`after_turn` (default):** preserves the existing behavior. A single result, - or one consolidated batch result, is delivered as a separate synthetic turn - after the foreground turn ends. +- **`after_turn` (default):** at each available turn boundary, every completed + but undelivered child from the batch is grouped into one synthetic turn. It + never waits for unfinished siblings: if 1/3 is ready, that result is delivered; + if two more are ready at a later boundary, those two are grouped into the next + turn. + +:::note Ready-set invariant +At one delivery boundary, one batch produces exactly one envelope containing +all of its currently completed, undelivered child rows. The envelope is claimed +and acknowledged atomically. Unfinished siblings are never waited on; children +that finish later form the ready-set at a later boundary. +::: + - **`inject`:** intended for auditors, reviewers, and dependent work that can change what the parent should do now. Each ready child is appended to the conversation at the next safe boundary, after all tool results from the @@ -149,7 +159,7 @@ delegate_task( ## Batch Mode Details -When a top-level agent provides a `tasks` array, Hermes returns one background handle and runs the subagents in parallel. With the default `after_turn` delivery it posts one consolidated result after every child finishes. With `inject`, each child summary can re-enter independently as soon as it is ready. An orchestrator subagent waits for its batch in the current turn so it can synthesize the results. +When a top-level agent provides a `tasks` array, Hermes returns one background handle and runs the subagents in parallel. With the default `after_turn` delivery, every ready child at an available turn boundary is grouped into one result turn; unfinished siblings do not block it and appear in a later ready-set. With `inject`, each child summary can re-enter independently as soon as it is ready. An orchestrator subagent waits for its batch in the current turn so it can synthesize the results. - **Maximum concurrency:** 3 tasks by default (configurable via `delegation.max_concurrent_children` or the `DELEGATION_MAX_CONCURRENT_CHILDREN` env var; floor of 1, no hard ceiling). Batches larger than the limit return a tool error rather than being silently truncated. - **Thread pool:** Uses `ThreadPoolExecutor` with the configured concurrency limit as max workers From e37a042debf72d6ae3dc627e680d974c83b09497 Mon Sep 17 00:00:00 2001 From: Xipong <217837358+Xipong@users.noreply.github.com> Date: Fri, 31 Jul 2026 01:35:22 +0300 Subject: [PATCH 7/7] fix(delegation): retry live completion leases --- agent/delegation_inject.py | 6 +- cli.py | 1 + gateway/run.py | 79 ++++++------ tests/agent/test_delegation_inject.py | 105 ++++++++++++++++ .../cli/test_cli_async_delegation_delivery.py | 32 +++++ tests/gateway/test_completion_delivery.py | 112 ++++++++++++++++++ tests/test_tui_gateway_server.py | 7 ++ tools/async_delegation.py | 95 +++++++++++++++ tools/process_registry.py | 94 +++++++++++++++ tui_gateway/server.py | 29 +++-- .../docs/user-guide/features/delegation.md | 12 +- 11 files changed, 522 insertions(+), 50 deletions(-) diff --git a/agent/delegation_inject.py b/agent/delegation_inject.py index 4ee9c5656106..2ceb71ed8829 100644 --- a/agent/delegation_inject.py +++ b/agent/delegation_inject.py @@ -345,8 +345,10 @@ def drain_ready_injects(agent: Any, messages: list[dict[str, Any]], turn_id: str claim_id = claim_event_delivery(event, f"conversation-loop:{os.getpid()}") if claim_id is None: - # A competing CLI/gateway process already owns this durable event, - # or it was delivered from a duplicate restored queue entry. + # A live lease can belong to a process that died shortly before + # this process restored the row. Defer until that lease expires; + # terminal duplicate rows are classified and discarded. + process_registry.defer_unclaimed_delivery(event) continue event_id = _event_identity(event) if _durable_event_is_in_history(messages, event_id): diff --git a/cli.py b/cli.py index 8cf25b13f568..69cd52e7432c 100644 --- a/cli.py +++ b/cli.py @@ -10343,6 +10343,7 @@ def _drain_process_notifications(self, consumer: str) -> None: ): claim = claim_event_delivery(event, consumer) if claim is None: + process_registry.defer_unclaimed_delivery(event) continue self._pending_input.put(synthetic_message) complete_event_delivery(event, claim) diff --git a/gateway/run.py b/gateway/run.py index 08f239248e4e..70cb8c93ee9e 100644 --- a/gateway/run.py +++ b/gateway/run.py @@ -21024,6 +21024,9 @@ async def _deliver_completion_notification( evt, f"gateway:{id(self)}" ) or "" if not durable_claim_id: + from tools.process_registry import process_registry + + process_registry.defer_unclaimed_delivery(evt) return None except Exception as exc: logger.warning( @@ -21165,8 +21168,9 @@ async def _async_delegation_watcher(self, interval: float = 2.0) -> None: # consume watch/completion events here (other drains own them), # so requeue anything that isn't ours. requeue = [] - async_events = [] + idle_events = [] with _pr.completion_routing_lock: + async_events = [] while not _pr.completion_queue.empty(): try: evt = _pr.completion_queue.get_nowait() @@ -21178,40 +21182,49 @@ async def _async_delegation_watcher(self, interval: float = 2.0) -> None: requeue.append(evt) for evt in requeue: _pr.completion_queue.put(evt) - from tools.async_delegation import coalesce_ready_after_turn_events - - async_events = coalesce_ready_after_turn_events(async_events) - for evt in async_events: - self._enrich_async_delegation_routing(evt) - # Busy-session routing: - # - inject for the matching active turn stays queued for the - # conversation loop's safe-boundary drain; - # - after_turn stays queued until that foreground turn ends. - # A requeued after-turn envelope is re-coalescible, so siblings - # that finish meanwhile join the same next-boundary delivery. - _rd = str(evt.get("result_delivery") or "after_turn").strip().lower() - _route_key = str(evt.get("session_key") or "").strip() - _event_turn_id = str(evt.get("parent_turn_id") or "") - _running_parent = getattr(self, "_running_agents", {}).get( - _route_key + + from tools.async_delegation import ( + coalesce_ready_after_turn_events, ) - if _running_parent is _AGENT_PENDING_SENTINEL: - _pr.completion_queue.put(evt) - continue - if _rd == "after_turn" and _running_parent is not None: - _pr.completion_queue.put(evt) - continue - if ( - _rd == "inject" - and _running_parent is not None - and _event_turn_id - and str( - getattr(_running_parent, "_active_turn_id", "") or "" + + async_events = coalesce_ready_after_turn_events(async_events) + for evt in async_events: + self._enrich_async_delegation_routing(evt) + # Busy-session routing is part of the same queue + # reservation as dequeue/coalescing. Otherwise the active + # conversation loop can observe a temporary-empty queue + # and miss an inject that was already ready at its safe + # boundary. + _rd = str( + evt.get("result_delivery") or "after_turn" + ).strip().lower() + _route_key = str(evt.get("session_key") or "").strip() + _event_turn_id = str(evt.get("parent_turn_id") or "") + _running_parent = getattr(self, "_running_agents", {}).get( + _route_key ) - == _event_turn_id - ): - _pr.completion_queue.put(evt) - continue + if _running_parent is _AGENT_PENDING_SENTINEL: + _pr.completion_queue.put(evt) + continue + if _rd == "after_turn" and _running_parent is not None: + _pr.completion_queue.put(evt) + continue + if ( + _rd == "inject" + and _running_parent is not None + and _event_turn_id + and str( + getattr(_running_parent, "_active_turn_id", "") or "" + ) + == _event_turn_id + ): + _pr.completion_queue.put(evt) + continue + idle_events.append(evt) + + # Formatting and adapter/network delivery must never hold the + # routing lock. Only events classified idle above leave it. + for evt in idle_events: synth_text = _format_gateway_process_notification(evt) if not synth_text: continue diff --git a/tests/agent/test_delegation_inject.py b/tests/agent/test_delegation_inject.py index d9625c70cd27..d19151b1a720 100644 --- a/tests/agent/test_delegation_inject.py +++ b/tests/agent/test_delegation_inject.py @@ -980,6 +980,28 @@ def test_restart_dedups_inject_already_persisted_in_active_history(): assert _event_state(delegation_id, "task:0") == ("delivered", 2) +def test_same_turn_claim_conflict_defers_pending_event(monkeypatch): + delegation_id = _record() + assert ad.publish_batch_child_completion( + delegation_id, 0, _child(0, "leased elsewhere") + ) + deferred = [] + monkeypatch.setattr(ad, "claim_event_delivery", lambda *_args: None) + monkeypatch.setattr( + process_registry, + "defer_unclaimed_delivery", + lambda evt: deferred.append(evt) or True, + ) + + messages = [{"role": "tool", "tool_call_id": "tc", "content": "done"}] + assert drain_ready_injects(SimpleNamespace(), messages, "turn-current") == 0 + + assert len(deferred) == 1 + assert deferred[0]["delegation_id"] == delegation_id + assert deferred[0]["delivery_event_key"] == "task:0" + assert len(messages) == 1 + + def test_formatter_failure_does_not_consume_delivery_attempts(monkeypatch): delegation_id = _record() assert ad.publish_batch_child_completion( @@ -1021,6 +1043,89 @@ def test_pending_child_event_restores_with_same_durable_identity(tmp_path, monke assert ad.restore_undelivered_completions(restored_queue) == 0 +def test_quick_restart_requeues_after_live_delivery_lease_expires( + tmp_path, monkeypatch +): + """A restored live-claimed row wakes without another process restart.""" + import queue as queue_module + + import tools.process_registry as registry_mod + + monkeypatch.setattr(ad, "_db_path", lambda: tmp_path / "state.db") + monkeypatch.setattr(ad, "_DELIVERY_CLAIM_LEASE_SECONDS", 0.15) + monkeypatch.setattr( + registry_mod, "CHECKPOINT_PATH", tmp_path / "processes.json" + ) + + delegation_id = _record(goals=("survive quick restart",)) + assert ad.publish_batch_child_completion( + delegation_id, 0, _child(0, "restored after lease") + ) + original = process_registry.completion_queue.get_nowait() + old_claim = ad.claim_event_delivery(original, "old-process") + assert old_claim + + restarted = registry_mod.ProcessRegistry() + restored = restarted.completion_queue.get_nowait() + assert restored["restored"] is True + assert ad.claim_event_delivery(restored, "new-process") is None + + assert restarted.defer_unclaimed_delivery(restored) is True + with pytest.raises(queue_module.Empty): + restarted.completion_queue.get_nowait() + + woke = restarted.completion_queue.get(timeout=1) + assert woke["delegation_id"] == delegation_id + assert woke["delivery_event_key"] == "task:0" + new_claim = ad.claim_event_delivery(woke, "new-process") + assert new_claim + assert ad.complete_event_delivery(woke, new_claim) + assert _event_state(delegation_id, "task:0") == ("delivered", 2) + assert restarted.defer_unclaimed_delivery(woke) is False + with pytest.raises(queue_module.Empty): + restarted.completion_queue.get_nowait() + + +def test_live_lease_retry_prunes_terminal_group_sibling(tmp_path, monkeypatch): + monkeypatch.setattr(ad, "_db_path", lambda: tmp_path / "state.db") + monkeypatch.setattr(ad, "_DELIVERY_CLAIM_LEASE_SECONDS", 0.15) + + delegation_id = _record( + goals=("already delivered", "still leased"), delivery="after_turn" + ) + assert ad.publish_batch_child_completion( + delegation_id, 0, _child(0, "delivered result") + ) + assert ad.publish_batch_child_completion( + delegation_id, 1, _child(1, "leased result") + ) + children = [ + process_registry.completion_queue.get_nowait(), + process_registry.completion_queue.get_nowait(), + ] + grouped = ad.coalesce_ready_after_turn_events(children)[0] + by_key = {event["delivery_event_key"]: event for event in children} + + delivered_claim = ad.claim_event_delivery(by_key["task:0"], "first-consumer") + assert delivered_claim + assert ad.complete_event_delivery(by_key["task:0"], delivered_claim) + live_claim = ad.claim_event_delivery(by_key["task:1"], "old-process") + assert live_claim + assert ad.claim_event_delivery(grouped, "new-process") is None + + assert process_registry.defer_unclaimed_delivery(grouped) + assert grouped["delivery_event_keys"] == ["task:1"] + assert [result["task_index"] for result in grouped["results"]] == [1] + + woke = process_registry.completion_queue.get(timeout=1) + assert woke["delivery_event_keys"] == ["task:1"] + retry_claim = ad.claim_event_delivery(woke, "new-process") + assert retry_claim + assert ad.complete_event_delivery(woke, retry_claim) + assert _event_state(delegation_id, "task:0") == ("delivered", 1) + assert _event_state(delegation_id, "task:1") == ("delivered", 2) + + def test_model_schema_defaults_after_turn_and_dispatch_forwards_explicit_mode(monkeypatch): delivery_schema = delegate_tool.DELEGATE_TASK_SCHEMA["parameters"]["properties"][ "result_delivery" diff --git a/tests/cli/test_cli_async_delegation_delivery.py b/tests/cli/test_cli_async_delegation_delivery.py index b970aca56a7f..907997a8aeea 100644 --- a/tests/cli/test_cli_async_delegation_delivery.py +++ b/tests/cli/test_cli_async_delegation_delivery.py @@ -47,6 +47,38 @@ def drain_notifications(self, *, session_key="", owns_event=None): assert completed == [(event, "claim-token")] +def test_cli_claim_conflict_defers_pending_durable_event(monkeypatch): + cli = HermesCLI.__new__(HermesCLI) + cli.session_id = "visible-session" + cli._pending_input = queue.Queue() + event = { + "type": "async_delegation", + "delegation_id": "deleg_live_lease", + "session_key": "visible-session", + } + deferred = [] + + class FakeRegistry: + def drain_notifications(self, *, session_key="", owns_event=None): + assert session_key == "visible-session" + assert owns_event is not None and owns_event(event) + return [(event, "completion payload")] + + def defer_unclaimed_delivery(self, evt): + deferred.append(evt) + return True + + monkeypatch.setattr("tools.process_registry.process_registry", FakeRegistry()) + monkeypatch.setattr( + "tools.async_delegation.claim_event_delivery", lambda *_args: None + ) + + cli._drain_process_notifications("cli-idle") + + assert deferred == [event] + assert cli._pending_input.empty() + + def test_cli_completion_ownership_rejects_foreign_session(): cli = HermesCLI.__new__(HermesCLI) cli.session_id = "visible-session" diff --git a/tests/gateway/test_completion_delivery.py b/tests/gateway/test_completion_delivery.py index ff8323736dea..3593e874785e 100644 --- a/tests/gateway/test_completion_delivery.py +++ b/tests/gateway/test_completion_delivery.py @@ -112,6 +112,118 @@ def test_duplicate_async_queue_replay_injects_once(monkeypatch, isolated_registr adapter.handle_message.assert_awaited_once() +def test_gateway_claim_conflict_defers_pending_durable_event( + monkeypatch, isolated_registry, +): + import tools.async_delegation as delegation_mod + + event = _async_event("deleg_gateway_live_lease") + deferred = [] + monkeypatch.setattr(delegation_mod, "claim_event_delivery", lambda *_args: None) + monkeypatch.setattr( + isolated_registry, + "defer_unclaimed_delivery", + lambda evt: deferred.append(evt) or True, + ) + + runner = _runner(SimpleNamespace(handle_message=AsyncMock())) + result = asyncio.run(runner._deliver_completion_notification("ready", event)) + + assert result is None + assert deferred == [event] + + +def test_gateway_busy_route_is_atomic_with_same_turn_inject_drain( + monkeypatch, isolated_registry, +): + """The watcher cannot hide a dequeued inject before active-turn requeue.""" + import threading + + import agent.delegation_inject as inject_mod + import tools.async_delegation as delegation_mod + import tools.process_registry as registry_mod + + isolated = queue.Queue() + monkeypatch.setattr(isolated_registry, "completion_queue", isolated) + monkeypatch.setattr(registry_mod, "_format_async_delegation", lambda _evt: "ready") + monkeypatch.setattr(inject_mod, "ensure_pending_inject_heartbeat", lambda _agent: True) + monkeypatch.setattr( + delegation_mod, + "claim_event_delivery", + lambda _event, _owner: "gateway-race-claim", + ) + + route_paused = threading.Event() + release_route = threading.Event() + original_coalesce = delegation_mod.coalesce_ready_after_turn_events + + def _pause_after_dequeue(events): + route_paused.set() + if not release_route.wait(3): + raise TimeoutError("test did not release paused gateway route") + return original_coalesce(events) + + monkeypatch.setattr( + delegation_mod, "coalesce_ready_after_turn_events", _pause_after_dequeue + ) + + turn_id = "turn-gateway-inject" + session_key = "agent:main:telegram:dm:12345:678" + active_agent = SimpleNamespace(_active_turn_id=turn_id) + event = { + **_async_event("deleg_gateway_inject_race"), + "delivery_event_key": "task:0", + "result_delivery": "inject", + "parent_turn_id": turn_id, + "session_key": session_key, + } + isolated.put(event) + + runner = _runner(SimpleNamespace(handle_message=AsyncMock())) + runner._running_agents = {session_key: active_agent} + _stop_after_sleeps(monkeypatch, runner, count=2) + + watcher_thread = threading.Thread( + target=lambda: asyncio.run(runner._async_delegation_watcher(interval=0)) + ) + messages = [{"role": "assistant", "content": "working"}] + drained = {} + drain_done = threading.Event() + + def _drain(): + drained["count"] = inject_mod.drain_ready_injects( + active_agent, messages, turn_id + ) + drain_done.set() + + drain_thread = threading.Thread(target=_drain) + try: + watcher_thread.start() + assert route_paused.wait(3), "gateway watcher did not reach post-dequeue route" + drain_thread.start() + + # Dequeue, coalesce, active-parent classification, and requeue must be one + # routing critical section. Returning here would degrade inject to late. + assert not drain_done.wait(0.5) + + release_route.set() + watcher_thread.join(3) + drain_thread.join(3) + + assert not watcher_thread.is_alive() + assert not drain_thread.is_alive() + assert drained == {"count": 1} + assert messages[-1]["content"] == "ready" + assert isolated.empty() + finally: + release_route.set() + runner._running = False + watcher_thread.join(3) + drain_thread.join(3) + while not isolated.empty(): + isolated.get_nowait() + + def test_gateway_watcher_coalesces_ready_after_turn_batch_children( monkeypatch, isolated_registry, ): diff --git a/tests/test_tui_gateway_server.py b/tests/test_tui_gateway_server.py index 9774fb3e6a1c..8d395ea26f99 100644 --- a/tests/test_tui_gateway_server.py +++ b/tests/test_tui_gateway_server.py @@ -13245,6 +13245,12 @@ def get(self, block=True, timeout=None): monkeypatch.setattr(process_registry, "completion_queue", isolated_queue) monkeypatch.setattr(registry_mod, "format_process_notification", lambda _evt: "ready") monkeypatch.setattr(delegation_mod, "claim_event_delivery", lambda *_args: None) + deferred = [] + monkeypatch.setattr( + process_registry, + "defer_unclaimed_delivery", + lambda evt: deferred.append(evt) or True, + ) monkeypatch.setattr(server, "_emit", lambda *_args, **_kwargs: None) sid = "sid-tui-claim-loss" @@ -13264,6 +13270,7 @@ def get(self, block=True, timeout=None): server._notification_poller_loop(stop_poller, sid, sess) assert sess["running"] is False + assert deferred == [event] assert isolated_queue.empty() finally: server._sessions.pop(sid, None) diff --git a/tools/async_delegation.py b/tools/async_delegation.py index 5075a4b65a06..89d743693025 100644 --- a/tools/async_delegation.py +++ b/tools/async_delegation.py @@ -1256,6 +1256,101 @@ def _retain_group_event_keys(evt: Dict[str, Any], event_keys: List[str]) -> None ) +def get_event_delivery_retry_delay( + evt: Dict[str, Any], *, now: Optional[float] = None +) -> Optional[float]: + """Return a bounded delay for an unclaimed pending event, else ``None``. + + A failed claim is ambiguous: another live process may own the pending row, + or this queue entry may be a duplicate whose durable row is already + delivered/dropped. Consumers use this oracle before deferred requeue so a + quick restart waits only for the remaining lease without spinning. Grouped + envelopes are narrowed to rows that are still pending. + """ + + if evt.get("type") != "async_delegation": + return None + delegation_id = str(evt.get("delegation_id") or "") + if not delegation_id: + return None + current = time.time() if now is None else float(now) + event_keys = _event_delivery_keys(evt) + pending_rows: List[tuple[str, Optional[str], Optional[float]]] = [] + + with _DB_LOCK, _transaction() as conn: + if "delivery_event_keys" in evt: + if not event_keys: + return None + placeholders = ",".join("?" for _ in event_keys) + rows = conn.execute( + f"""SELECT event_key, delivery_state, delivery_claim, + delivery_claimed_at + FROM async_delegation_events + WHERE delegation_id=? AND event_key IN ({placeholders})""", + (delegation_id, *event_keys), + ).fetchall() + by_key = {str(row[0]): row for row in rows} + if len(by_key) != len(event_keys): + return None + pending_keys = [ + key for key in event_keys if str(by_key[key][1]) == "pending" + ] + if not pending_keys: + return None + _retain_group_event_keys(evt, pending_keys) + pending_rows = [ + ( + key, + str(by_key[key][2]) if by_key[key][2] else None, + float(by_key[key][3]) if by_key[key][3] is not None else None, + ) + for key in pending_keys + ] + elif event_keys: + row = conn.execute( + """SELECT delivery_state, delivery_claim, delivery_claimed_at + FROM async_delegation_events + WHERE delegation_id=? AND event_key=?""", + (delegation_id, event_keys[0]), + ).fetchone() + if row is None or str(row[0]) != "pending": + return None + pending_rows = [ + ( + event_keys[0], + str(row[1]) if row[1] else None, + float(row[2]) if row[2] is not None else None, + ) + ] + else: + row = conn.execute( + """SELECT delivery_state, delivery_claim, delivery_claimed_at + FROM async_delegations WHERE delegation_id=?""", + (delegation_id,), + ).fetchone() + if row is None or str(row[0]) != "pending": + return None + pending_rows = [ + ( + "aggregate", + str(row[1]) if row[1] else None, + float(row[2]) if row[2] is not None else None, + ) + ] + + # A short guard covers claim races and strict SQL '<' lease comparison. + delay = 0.05 + for _event_key, claim, claimed_at in pending_rows: + if not claim: + continue + if claimed_at is None: + delay = max(delay, float(_DELIVERY_CLAIM_LEASE_SECONDS)) + continue + remaining = float(_DELIVERY_CLAIM_LEASE_SECONDS) - (current - claimed_at) + delay = max(delay, remaining) + return max(0.05, delay) + 0.05 + + def complete_event_delivery(evt: Dict[str, Any], claim_id: str) -> bool: if not claim_id or evt.get("type") != "async_delegation": return False diff --git a/tools/process_registry.py b/tools/process_registry.py index c3c2402ad09d..b37f6f56aa56 100644 --- a/tools/process_registry.py +++ b/tools/process_registry.py @@ -29,6 +29,7 @@ process_registry.kill(session.id) """ +import heapq import json import logging import os @@ -178,6 +179,15 @@ def __init__(self): # that route/claim completion events hold this lock only for the bounded # dequeue -> decision handoff, never while an agent/model turn runs. self.completion_routing_lock = threading.RLock() + # Durable events that lose a claim race must not disappear until the + # next process restart, but immediate requeue would make fast TUI + # pollers spin. One lazy daemon scheduler owns all delayed retries for + # this registry. Exact durable identities are deduplicated in the heap. + self._deferred_completion_condition = threading.Condition() + self._deferred_completion_heap: list[tuple[float, int, tuple, dict]] = [] + self._deferred_completion_deadlines: dict[tuple, float] = {} + self._deferred_completion_sequence = 0 + self._deferred_completion_thread: Optional[threading.Thread] = None # Rehydrate durable delegation completions only at registry startup. # Consumers still inject them as fresh turns through this existing rail. try: @@ -1239,6 +1249,90 @@ def _drain_should_skip( skip_poll_observed and session_id in self._poll_observed ) + @staticmethod + def _deferred_completion_key(event: dict) -> tuple: + raw_keys = event.get("delivery_event_keys") + if isinstance(raw_keys, (list, tuple)): + event_keys = tuple(str(key) for key in raw_keys if key) + else: + event_key = str(event.get("delivery_event_key") or "") + event_keys = (event_key,) if event_key else ("aggregate",) + return ( + "async_delegation", + str(event.get("delegation_id") or ""), + event_keys, + ) + + def defer_unclaimed_delivery(self, event: dict) -> bool: + """Requeue a pending durable event after its competing lease expires. + + ``claim_event_delivery()`` returning ``None`` is not enough to drop the + RAM copy: after a quick restart the old process's lease can still be + live. Terminal duplicates return ``False`` and disappear; pending rows + enter one deduplicated heap and wake under the routing lock. + """ + + try: + from tools.async_delegation import get_event_delivery_retry_delay + + delay = get_event_delivery_retry_delay(event) + except Exception: + logger.exception("Could not classify unclaimed delegation event") + return False + if delay is None: + return False + + key = self._deferred_completion_key(event) + if not key[1]: + return False + deadline = time.monotonic() + max(0.05, float(delay)) + with self._deferred_completion_condition: + existing = self._deferred_completion_deadlines.get(key) + if existing is not None and existing <= deadline: + return True + self._deferred_completion_sequence += 1 + self._deferred_completion_deadlines[key] = deadline + heapq.heappush( + self._deferred_completion_heap, + ( + deadline, + self._deferred_completion_sequence, + key, + event, + ), + ) + thread = self._deferred_completion_thread + if thread is None or not thread.is_alive(): + thread = threading.Thread( + target=self._deferred_completion_loop, + name="completion-lease-retry", + daemon=True, + ) + self._deferred_completion_thread = thread + thread.start() + self._deferred_completion_condition.notify() + return True + + def _deferred_completion_loop(self) -> None: + while True: + with self._deferred_completion_condition: + while not self._deferred_completion_heap: + self._deferred_completion_condition.wait() + deadline, _sequence, key, event = self._deferred_completion_heap[0] + current = self._deferred_completion_deadlines.get(key) + if current != deadline: + heapq.heappop(self._deferred_completion_heap) + continue + remaining = deadline - time.monotonic() + if remaining > 0: + self._deferred_completion_condition.wait(timeout=remaining) + continue + heapq.heappop(self._deferred_completion_heap) + self._deferred_completion_deadlines.pop(key, None) + + with self.completion_routing_lock: + self.completion_queue.put(event) + def collect_ready_after_turn_siblings(self, seed: dict) -> dict: """Fold queued ready siblings into one transient after-turn envelope. diff --git a/tui_gateway/server.py b/tui_gateway/server.py index b57508d6eed5..8e44ee3b0a5b 100644 --- a/tui_gateway/server.py +++ b/tui_gateway/server.py @@ -8754,11 +8754,12 @@ def _notification_poller_loop( _claim = claim_event_delivery(evt, "tui-poller") if _claim is None: - # The active loop or another durable - # consumer won the event. Do not mark - # this TUI session busy for work it - # will never dispatch. - _route_action = "claimed_elsewhere" + # Keep a live-claimed restored row out + # of the hot poll loop until its lease + # expires. Terminal duplicates are + # discarded by the durable classifier. + process_registry.defer_unclaimed_delivery(evt) + _route_action = "deferred_claim" else: session["running"] = True _route_action = "dispatch" @@ -8868,7 +8869,9 @@ def _notification_poller_loop( from tools.async_delegation import claim_event_delivery _claim = claim_event_delivery(evt, "tui-poller") - if _claim is not None: + if _claim is None: + process_registry.defer_unclaimed_delivery(evt) + else: session["running"] = True _shutdown_action = "dispatch" @@ -9759,17 +9762,21 @@ def _interim_assistant_cb(text: str, *, already_streamed: bool = False) -> None: owns_event=lambda e: _session_owns_notification_event(sid, session, e), skip_poll_observed=False, ) + from tools.async_delegation import ( + claim_event_delivery, complete_event_delivery, release_event_delivery, + ) for index, (_evt, synth) in enumerate(drained): + _claim = None with session["history_lock"]: if session.get("running"): for pending_evt, _pending_synth in drained[index:]: process_registry.completion_queue.put(pending_evt) break - session["running"] = True - from tools.async_delegation import ( - claim_event_delivery, complete_event_delivery, release_event_delivery, - ) - _claim = claim_event_delivery(_evt, "tui-post-turn") + _claim = claim_event_delivery(_evt, "tui-post-turn") + if _claim is None: + process_registry.defer_unclaimed_delivery(_evt) + else: + session["running"] = True if _claim is None: continue try: diff --git a/website/docs/user-guide/features/delegation.md b/website/docs/user-guide/features/delegation.md index 10f90d6680b7..1abe64c557e4 100644 --- a/website/docs/user-guide/features/delegation.md +++ b/website/docs/user-guide/features/delegation.md @@ -173,13 +173,17 @@ Synchronous single-task delegation from an orchestrator runs directly without th When a background delegation finishes, Hermes stores its completion event in the active profile's `state.db` before publishing it to the shared completion -queue. `inject` batches use one execution record plus independently claimable -child-delivery records; `after_turn` keeps one aggregate delivery record. If +queue. Both `inject` and `after_turn` batches use one execution record plus +independently claimable child-delivery records. At an idle `after_turn` boundary, +currently-ready child rows are folded into a transient grouped envelope; the +envelope is not a second aggregate database row. Legacy aggregate records from +older Hermes versions remain deliverable through the compatibility path. If Hermes restarts after completion but before delivery, pending events are restored and routed through the same ownership checks. Competing consumers use a durable claim, so only the consumer that successfully appends or accepts the -synthetic turn acknowledges each event; failed attempts release the claim for -retry. +synthetic turn acknowledges each event. Failed attempts release the claim for +retry; a restored row whose previous process still owns a live lease is +rescheduled for the lease boundary without spinning the completion queue. This does not resume child execution after a crash. A delegation whose owner process disappears while it is still running is recorded as `unknown`, because