fix(delegation): honor explicit background false - #55010
Conversation
|
@teknium1 since this touches the behavior introduced around async top-level delegation, I’d appreciate your design read here. I agree async-by-default is useful for interactive chat, but I think there’s still a legitimate same-turn orchestration use case outside cron: e.g. “before returning, ask another subagent/model to challenge this recommendation and incorporate the critique into the final answer.” That pattern needs the parent to explicitly wait before finalizing. This draft tries to preserve the async default while restoring |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for isolating the same-turn delegation case. The premise is verified on current main: run_agent.py:5706-5724 explicitly forces top-level model calls to background mode, and tools/delegate_tool.py:3460-3473 mirrors that behavior in the registry fallback.
Problems
- The new docs example at
website/docs/user-guide/features/delegation.md:29passestoolsets, but current model-facing schema properties omit that field (tools/delegate_tool.py:3410-3449) and dispatch does not forward it (run_agent.py:5718-5725).
Suggested changes
- Remove
toolsetsfrom that new example or document inherited toolsets. - If this behavior is adopted, port the narrow background-resolution change onto current dispatch while retaining
_strip_model_hidden_task_fieldsatrun_agent.py:5721; current main removed model-controlled ACP transport ine4dbb67bf5.
Automated hermes-sweeper review.
| ```python | ||
| delegate_task( | ||
| goal="Critique this provisional recommendation before I finalize", | ||
| context="...", |
There was a problem hiding this comment.
toolsets is not accepted by the current model-facing delegate_task schema or forwarded by its dispatcher (tools/delegate_tool.py:3410-3449, run_agent.py:5718-5725). Remove it here or explain that subagents inherit the parent's toolsets.
|
Thanks — I’ve addressed both points and updated the branch against current
Targeted verification:
The PR is now conflict-free and mergeable. |
What does this PR do?
Restores explicit
background=falsehandling in the model-facingdelegate_taskdispatch path while keeping top-level delegation asynchronous by default.Current
run_agent._dispatch_delegate_task()forces top-level model delegations tobackground=True, and the tool schema says thebackgroundparameter is ignored. That default is useful for interactive chats, but it removes the same-turn orchestration path needed by cron jobs and other workflows that must wait for a subagent before producing one final response.This PR keeps the responsive async default while restoring an explicit escape hatch:
Behavior after this change:
backgroundargbackground=Truebackground=False_delegate_depth > 0)delegate_task()The current model-facing ACP security boundary remains intact: hidden per-task
acp_commandandacp_argsfields are still removed before dispatch.Related Issue
Related: #53062
That PR addresses cron-session delivery for async delegation. This PR covers the narrower API/control question: preserve async-by-default behavior while honoring explicit
background=falsefor callers that need same-turn orchestration.Type of Change
Changes Made
run_agent.py: keep top-level model delegation async by default, but honor explicitbackground=false; retain_strip_model_hidden_task_fields()and keep orchestrator/subagent delegation synchronous.tools/delegate_tool.py: mirror the same background resolution in the registry fallback and update the model-facing schema description.tests/tools/test_async_delegation.py: cover default async behavior, explicitbackground=falsefor single and batch calls, and forced synchronous behavior for nested/orchestrator callers.website/docs/user-guide/features/delegation.md: document the async default, same-turn escape hatch, batch behavior, and process-local durability semantics.How to Test
Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests passDocumentation & Housekeeping
docs/, docstrings) — or N/Acli-config.yaml.exampleif I added/changed config keys — N/A: no config key addedCONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — N/A: no contributor workflow changeScreenshots / Logs
N/A — behavior is covered by unit tests and documentation updates.