feat(delegation): add delegate_tool_reply explicit delivery channel f… - #61332
feat(delegation): add delegate_tool_reply explicit delivery channel f…#61332DarkMagicCK wants to merge 3 commits into
Conversation
…or subagents Subagents that emit their deliverable on a turn that also calls a cleanup tool (e.g. terminal rm) lose it: the loop treats that content as mid-task narration, and a later short closing comment overwrites final_response. Heuristics (length, tool category) can't reliably distinguish deliverable from narration — the 0/1-judgment + long-cleanup counterexample proves it. Add an explicit delivery channel: delegate_tool_reply, a leaf-subagent-only tool that hands back the result via a structured tool call. The parent's extraction reads the tool args (authoritative) instead of guessing from final_response. Falls back to final_response when the child never calls it (strictly not-worse-than-status-quo). Visibility (zero core footprint, kanban toolset precedent): - delegation_reply toolset not in _HERMES_CORE_TOOLS / CONFIGURABLE_TOOLSETS - _build_child_agent appends it to every spawned child's toolset - ordinary conversations never see the schema; hermes tools / /tools hide it Extraction priority in _extract_reply_deliverable: 1. spill file (handler writes cache/delegation/delegate_reply_*.txt) — complete, immune to context-compression args truncation 2. tool-call args content (last call in protected tail = intact) 3. multi-call concat in order; truncated-only calls get a marker 4. no call -> final_response (unchanged) Compression resilience: handler spills every call to disk and returns the path; extraction prefers the file over possibly-truncated args. Handler runs in the agent process (like todo/memory), not the terminal sandbox, so the spill lands on the agent host reachable by the parent's read_file regardless of remote (docker/ssh/modal) terminal backends.
Related: #58490 (the subagent-output-loss bug this addresses), #28453 / #28431 (prior attempts to expose structured per-turn/multi-turn response metadata from run_conversation() — a competing mechanism, not a duplicate), #55010 (delegate-background family). This PR introduces a dedicated |
|
Thanks for isolating the delegated-child result-loss path; current main still takes the child summary only from Problems
Suggested changes
Automated hermes-sweeper review. |
Address teknium1 review on PR NousResearch#61332: 1. Compression resilience (the core issue): the previous extractor scanned result["messages"] for delegate_tool_reply tool calls + spill paths. But context_compressor.py Phase 4 replaces the entire middle transcript with a summary - a delivery call that falls into the compacted window loses both its args AND its tool-result spill path. Fix: the handler now appends content to child._delegate_reply_chunks (agent-instance state, outside messages[]) at execution time. _extract_reply_deliverable reads from the agent instance, not the transcript. Compression cannot touch it. 2. Schema semantics: removed contradictory update/replace wording. The schema now states append-only: every call content is concatenated in order. The extractor matches - pure append, no replace. 3. Tests: replaced the hand-written child_toolsets.append() test with the constructor-capture pattern from test_delegate.py:1837 (patch run_agent.AIAgent, call _build_child_agent, assert enabled_toolsets contains delegation_reply). Added compression regression test proving the deliverable survives when messages is replaced by a synthetic summary.
Thanks for the review, the new commit address teknium1 review on PR #61332:
|
delegate_tool_reply was routed through registry.dispatch which never forwards parent_agent — the handler received None and only produced spill files with 'unknown' subagent ids, never recording chunks on the agent instance. _run_single_child then found empty _delegate_reply_chunks and fell back to trailing final_response prose. Fix: intercept delegate_tool_reply inline in both dispatch paths (tool_executor.py sequential + agent_runtime_helpers.invoke_tool concurrent), passing the agent instance directly — same pattern as todo/memory/clarify/ delegate_task. Add to AGENT_RUNTIME_POST_HOOK_TOOL_NAMES for post-hook ownership tracking. Tests: fix 2 existing MagicMock tests (SimpleNamespace for real attribute semantics), add 2 dispatch-path tests proving registry loses the agent reference while agent-level interception preserves it.
Address teknium1 review on PR NousResearch#61332: 1. Compression resilience (the core issue): the previous extractor scanned result["messages"] for delegate_tool_reply tool calls + spill paths. But context_compressor.py Phase 4 replaces the entire middle transcript with a summary - a delivery call that falls into the compacted window loses both its args AND its tool-result spill path. Fix: the handler now appends content to child._delegate_reply_chunks (agent-instance state, outside messages[]) at execution time. _extract_reply_deliverable reads from the agent instance, not the transcript. Compression cannot touch it. 2. Schema semantics: removed contradictory update/replace wording. The schema now states append-only: every call content is concatenated in order. The extractor matches - pure append, no replace. 3. Tests: replaced the hand-written child_toolsets.append() test with the constructor-capture pattern from test_delegate.py:1837 (patch run_agent.AIAgent, call _build_child_agent, assert enabled_toolsets contains delegation_reply). Added compression regression test proving the deliverable survives when messages is replaced by a synthetic summary.
Summary
When a subagent produces its result on the same turn it calls a tool (e.g. writing a file then calling
terminalto clean up a temporary script), the deliverable is silently lost. The conversation loop treats content emitted alongside tool calls as mid-task narration (_last_content_with_tools,conversation_loop.py:4613) rather than a final answer, so it never becomesfinal_response. A later turn where the subagent says something short — like "done, cleaned up, no files changed" — becomesfinal_responseand the parent agent only receives that closing comment. The actual work product never reaches the caller.A concrete example: you delegate an audit task to a subagent. The subagent runs its analysis, writes the full findings as its reply text, then calls
terminalto delete the throwaway script it used. On the next turn it adds "the task is complete, nothing else to report." The parent agent's summary ends up being just that one-line closer — the entire audit is gone.This is the failure reported in #28326 (Lark + streaming) and is the class of bug that #28453 attempted to address by adding a
content_segmentsmetadata field to the result dict.Why heuristics can't fix this
The existing fallback in
conversation_loop.py:4827-4851only promotes prior content when all tool calls in the turn are housekeeping tools (memory,todo,skill_manage,session_search) and the follow-up is empty. Two counterexamples prove no content-feature heuristic is sound:terminal(rm).terminalis substantive (not in_HOUSEKEEPING_TOOLS), so the fallback never fires even if we relaxed the "empty follow-up" gate.The root cause is an implicit delivery channel —
final_responseis a trailing-prose proxy, and the agent loop cannot distinguish "this is the deliverable" from "this is narration before I go clean up." The fix is to give the subagent an explicit delivery channel.The fix:
delegate_tool_replyA leaf-subagent-only tool that hands the deliverable back through a structured tool call. The parent's extraction reads the tool-call args (authoritative) instead of guessing from
final_response. Falls back tofinal_responsewhen the child never calls the tool — strictly not-worse-than-status-quo.Extraction priority (
_extract_reply_deliverable,delegate_tool.py:1599)contenttocache/delegation/delegate_reply-*.txtand returns the path; extraction reads the file (complete, compression-immune).content(intact for calls in the protected tail; may be truncated for older calls).max_tokens).final_responseunchanged.When any
delegate_tool_replycontent is assembled, it replacesfinal_responseas the summary.Why this approach over #28453's
content_segments#28453 proposed adding a
content_segmentslist to the result dict so callers could access every assistant content block. It was rejected (stale branch, wrong premise) and the approach has a fundamental problem: no consumer reads it. The result dict is assembled inturn_finalizer.py, notconversation_loop.py, and no gateway/platform caller readscontent_segments. Adding a metadata field with no extraction path doesn't fix the bug — the parent still getsfinal_response.delegate_tool_replysolves this by making the deliverable a tool call that the existing extraction layer (delegate_tool.py:2037) already scans (the tool-trace builder atdelegate_tool.py:2068-2075iterates every assistant tool_call). The fix is wired into the real return path, not an unplumbed metadata field.Visibility — zero core footprint
delegation_replyis a separate toolset, not inCONFIGURABLE_TOOLSETS(tools_config.py:60-86) so it never appears inhermes tools//tools. Unlike thekanbantoolset (whose tool names are in_HERMES_CORE_TOOLSattoolsets.py:70-77and are gated bycheck_fnviaHERMES_KANBAN_TASK),delegation_replyis also excluded from_HERMES_CORE_TOOLS— no platform bundle ever auto-includes it._build_child_agent(delegate_tool.py:1166) appends it to every spawned child's toolset, so the schema only appears on subagents. Ordinary conversations never see the name; users never see a toggle.This is Footprint Ladder rung 3 (service-gated tool) — narrower than kanban's env-gated rung, since it needs no
check_fnand no env var. Visibility is purely toolset membership driven per-agent byenabled_toolsets.Compression resilience
If a long-running subagent triggers context compression, the compressor truncates
tool_callargs > 500 chars for assistant messages outside the protected tail (context_compressor.py:1411-1429,_truncate_tool_call_args_jsonat:435). This would destroy a large deliverable stored only in args.The handler spills every call to
cache/delegation/(mirroring the existing_spill_summary_to_fileatdelegate_tool.py:1568), returning the abs path. Extraction prefers the file over possibly-truncated args. The spill dir is mounted read-only into remote backends (Docker/Modal/SSH) viacredential_files._CACHE_DIRS, so the parent'sread_filecan page through on any backend.The handler runs in the agent's Python process (like
todo/memory), not in the terminal sandbox — so the spill lands on the agent host regardless of whether the child'sterminalpoints at a remote Docker/SSH/Modal backend. There is no separate "subagent host"; the child runs in aDaemonThreadPoolExecutorworker thread inside the parent process.System prompt discipline
_build_child_system_prompt(delegate_tool.py:705) injects a Delivery Discipline block into every child's system prompt, instructing the subagent to calldelegate_tool_replywith its result and not rely on trailing prose. This is auto-injected — not reliant on the parent hand-writing it incontext.Changes
tools/delegate_tool_reply.py(new)registry.registerunderdelegation_replytoolsettoolsets.pydelegation_replytoolset entry (:251); not in_HERMES_CORE_TOOLStools/delegate_tool.py_build_child_agentinjects toolset (:1166); system prompt discipline (:705);_extract_reply_deliverable(:1599) + extraction wiring (:2167)tests/tools/test_delegate_tool_reply.py(new)tests/tools/test_delegate.pydelegation_replytoolset memberScope — what this does NOT fix (intentional)
turn_finalizer.py) — separate bug, separate PR. The extraction change here is indelegate_tool.py, upstream of finalizer persistence._last_content_with_tools— the general heuristic unsoundness (narration vs. deliverable in normal chats) is a larger design question. This PR scopes todelegate_taskwhere the deliverable contract is unambiguous.Test plan
tests/tools/test_delegate_tool_reply.py)ruff checkclean on all changed filesdelegate_taskwhose goal ends with "clean up tmp files, then reply" — assert parent summary contains the deliverable, not a cleanup comment