Skip to content

fix(delegate): declare stateless channel in one-shot and cron so delegate_task returns results - #63866

Closed
cgarwood82 wants to merge 1 commit into
NousResearch:mainfrom
cgarwood82:fix/stateless-delegation-oneshot-cron
Closed

fix(delegate): declare stateless channel in one-shot and cron so delegate_task returns results#63866
cgarwood82 wants to merge 1 commit into
NousResearch:mainfrom
cgarwood82:fix/stateless-delegation-oneshot-cron

Conversation

@cgarwood82

Copy link
Copy Markdown
Contributor

What does this PR do?

run_agent._dispatch_delegate_task forces background=True for every top-level delegation, and async_delivery_supported() returns True for any session that never binds the capability. On a runner that cannot receive a completion after its turn ends, that combination silently discards every subagent result — the model gets a dispatch handle, ends its turn, and reports "waiting for results".

Two runners never bind the capability:

  • hermes -z (one-shot) prints one final response and exits. It bypasses cli.py, so nothing drains process_registry.completion_queue (only the interactive process_loop and the gateway watchers do).
  • cron run_job deliberately clears the HERMES_SESSION_* routing keys, so a completion event carries session_key=""_enrich_async_delegation_routing cannot resolve it and _inject_watch_notification drops it ("no routing metadata"). By then run_job has already shipped the job's final response via _deliver_result; there is no turn left to re-enter. Worse, get_current_session_key() can fall back to the ambient os.environ["HERMES_SESSION_KEY"], so a cron subagent's output can be routed into an unrelated user chat rather than merely dropped.

This adds declare_stateless_channel() and binds it in both runners, routing delegate_task to its existing inline/synchronous path — the same fallback the stateless HTTP adapter already relies on, and the fix suggested in #63142.

This is not new behavior. It restores the documented default. From website/docs/user-guide/features/delegation.md (added in af250d8):

By default, delegate_task runs inside the parent's current turn and blocks until every child finishes. With background=true, the child may continue after that turn returns while the owning session and Hermes process remain alive.

For one-shot the process does not remain alive, and for cron there is no resolvable owning session — so background=true is a promise neither channel can keep.

Note: this reproduces on current main (af250d8), after the durable-completion work in 67f4e1b / d0e9a42. Those commits correctly store the completion; nothing on these two paths ever consumes it.

Related Issue

Fixes #53027
Fixes #63142

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • gateway/session_context.py — add declare_stateless_channel(). Binds only the capability; set_session_vars() would also latch _session_context_engaged, which switches the subprocess env bridge to ContextVar-authoritative — a side effect a pure single-process one-shot must not trigger. Updated async_delivery_supported()'s docstring, which claimed cron was always supported.
  • hermes_cli/oneshot.py — call it alongside the other process-wide setup in run_oneshot().
  • cron/scheduler.pyrun_job passes async_delivery=False to its existing set_session_vars() call.
  • tools/delegate_tool.py, tools/terminal_tool.py — the two agent-facing strings hardcoded "stateless HTTP API" as the only channel without async delivery. They now name the actual condition, so a cron job is no longer told it is an HTTP endpoint.
  • tests/gateway/test_async_delivery_capability.py — new tests (below).

How to Test

Repro on main:

hermes -z "Call delegate_task to spawn a subagent that replies with the single word BANANA. Report its exact reply."
# -> "(Waiting for the subagent's reply…)", exit 0, no BANANA

With this PR: BANANA is returned in-turn.

Tests added to tests/gateway/test_async_delivery_capability.py:

  • test_declare_stateless_channel_disables_async_delivery
  • test_declare_does_not_engage_full_session_context — pins the reason the helper exists rather than reusing set_session_vars
  • test_background_delegation_runs_inline_when_channel_is_stateless — the behavioral contract: asserts dispatch_async_delegation_batch is not called and the child's result comes back in the payload

Behavior changes worth calling out

Cron delegations now block. delegation.child_timeout_seconds defaults to no timeout, and the delegation heartbeat refreshes the parent's activity timestamp, so cron's inactivity watchdog cannot fire until the child's heartbeat goes stale. Worst case is bounded but long. A workdir job also holds _terminal_cwd_lock as a writer for its whole run, so one delegating workdir job blocks other cron jobs for that window. This is the pre-0.18.2 behavior described in #63142 ("delegate_task completed (402.71s, 15,075 chars) ✓ sync, real results"), and it is strictly better than returning placeholder text — but it is a change, and installs relying on cron jobs returning promptly should set child_timeout_seconds.

This avoids #63769 rather than aggravating it. The stateless branch returns before dispatch_async_delegation_batch, so these runners never touch the shared async pool and never reach the pool-at-capacity fallback where that bug lives.

Known residual (out of scope). _run_single_child submits without a context copy, so subagents start with the capability unset. A subagent calling terminal(notify_on_complete=True) inside a cron/one-shot run is still told async works and its notification is still dropped. Happy to address in a follow-up.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix
  • I've run the full suite via scripts/run_tests.sh40,646 passed, 4 failed. The 4 (test_bedrock_integration, test_startup_restart_race, test_gateway_runtime_health, test_resolve_provider_openrouter_pool) fail identically on unmodified main and are unrelated to this change.
  • I've added tests for my changes
  • I've tested on my platform: Arch Linux, Python 3.12

Documentation & Housekeeping

  • Docstrings updated (declare_stateless_channel, async_delivery_supported) — the latter previously documented cron as async-capable, which is what made this bug easy to miss
  • No config keys added — N/A
  • No architecture/workflow change — N/A
  • Cross-platform: no platform-specific code; ContextVar binding is stdlib
  • Tool descriptions/schemas: two agent-facing strings corrected (see Changes Made)

…gate_task returns results

run_agent._dispatch_delegate_task forces background=True for every top-level
delegation, and async_delivery_supported() returns True for any session that
never binds the capability. On runners that cannot receive a completion after
their turn ends, that combination silently discards every subagent result: the
model gets a dispatch handle, ends its turn, and reports 'waiting for results'.

Two such runners never bind the capability:

* hermes -z (one-shot) prints one final response and exits. It bypasses cli.py,
  so nothing drains process_registry.completion_queue (only the interactive
  process_loop and the gateway watchers do).

* cron run_job clears the HERMES_SESSION_* routing keys, so a completion event
  carries session_key="" — _enrich_async_delegation_routing cannot resolve it
  and _inject_watch_notification drops it ("no routing metadata"). By then
  run_job has already shipped the job's final response via _deliver_result;
  there is no turn left to re-enter. Worse, get_current_session_key() can fall
  back to the ambient os.environ HERMES_SESSION_KEY, so a cron subagent's output
  can be routed into an unrelated user chat rather than merely dropped.

Add declare_stateless_channel() and bind it in both runners, routing
delegate_task to its existing inline/synchronous path — the same fallback the
stateless HTTP adapter already relies on, and the fix suggested in NousResearch#63142. The
helper binds only the capability: set_session_vars() would also latch
_session_context_engaged, which a pure single-process one-shot must not trigger.

Also correct two agent-facing strings that hardcoded 'stateless HTTP API' as the
only channel without async delivery (delegate_tool, terminal_tool); they now name
the actual condition.

Repro (before): hermes -z 'Use delegate_task to spawn a subagent that replies
BANANA. Report its reply.' -> "Waiting for the subagent's response...", exit 0,
no BANANA. After: BANANA is returned in-turn.

Fixes NousResearch#53027
Fixes NousResearch#63142
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint tool/delegate Subagent delegation comp/cron Cron scheduler and job management sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Jul 13, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for tracing this to the delivery-capability gate. Current main forces top-level delegation into background mode (run_agent.py:5798-5816), while cron binds its context without opting out (cron/scheduler.py:2839-2843), so the existing synchronous fallback in tools/delegate_tool.py:2812-2825 is the right mechanism.

Problems

  • The new cron comment says get_current_session_key() can fall back to ambient HERMES_SESSION_KEY. In this cron path, set_session_vars() explicitly sets the session key to "" (cron/scheduler.py:2839-2843; gateway/session_context.py:201), and get_session_env() does not fall back when a ContextVar is explicitly set (gateway/session_context.py:321-327). The dropped completion rationale remains valid; this parenthetical should be removed or corrected.
  • The new tests exercise the helper and direct delegate_task fallback, but not either changed runner. A regression test should observe async_delivery_supported() == False from inside run_oneshot()'s _run_agent seam and cron's AIAgent execution path.

Suggested changes

  • Correct the cron routing comment and matching PR explanation.
  • Add the two runner-wiring tests while retaining the direct fallback contract test.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 16, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Merged as #66617 (rebased so your commit keeps authorship). Yours was the earliest and the only complete fix in the cluster — it covers one-shot AND cron AND the terminal watcher path, and it correctly uses a dedicated declare_stateless_channel() that binds only the delivery capability without tripping the _session_context_engaged latch (which set_session_vars would). Verified with your test suite (15/15) plus a live E2E confirming a background=true batch in a stateless channel falls back to synchronous and returns results inline. Thanks @cgarwood82 — this closes #53027.

@teknium1 teknium1 closed this Jul 18, 2026
klopyrev added a commit to klopyrev/hermes-agent that referenced this pull request Jul 25, 2026
…ainer)

Bug: on the ACP surface, delegate_task silently loses every subagent
result. The dispatch succeeds and the child runs to completion, but its
summary never re-enters the conversation; the model tells the user to
wait for a result that cannot arrive.

Root cause: async delegation completions are pushed onto the process-
global process_registry.completion_queue, and delivery is each surface's
job — CLI drains between prompts, TUI/desktop run a notification poller,
the gateway has drain loops. acp_adapter/ has NO consumer for that queue,
and additionally binds its session context via set_session_vars() with
the default async_delivery=True, so delegate_task's capability gate
(async_delivery_supported()) wrongly permits background dispatch on a
channel that can never deliver. The completion event rots in the in-
memory queue until the hermes acp process exits.

Fix: bind ACP sessions with async_delivery=False, the same contract the
stateless API server declares (supports_async_delivery=False; cf.
upstream PRs NousResearch#50319, NousResearch#63866 for the api-server and cron surfaces).
delegate_task then takes its existing synchronous fallback: children run
inside the tool call and their summaries return in the tool result,
within the same turn.

Behavior change: ACP delegations now block the prompt until the
subagents finish (batch tasks still run in parallel; the call waits for
all). Cancelling the turn interrupts attached children via the sync
path's normal lifecycle.

Upstream: tracked as issue NousResearch#62548; open PR NousResearch#62558 adds a real drainer
but delivers results as display-only session/update chunks that never
re-enter the model's context, so this local override remains preferable
for agent-consumable delegation results even if that PR lands. Bug
report: ~/config/hermes/bug_reports/acp-delegate-task-results-never-delivered.md

Regression test: tests/acp/test_server.py::TestPrompt::
test_prompt_binds_async_delivery_unsupported captures
async_delivery_supported() from inside run_conversation (the exact
context where delegate_task reads the flag); verified to FAIL against
the pre-fix binding and PASS with the fix.
@cgarwood82
cgarwood82 deleted the fix/stateless-delegation-oneshot-cron branch August 5, 2026 04:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/cron Cron scheduler and job management P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state tool/delegate Subagent delegation type/bug Something isn't working

Projects

None yet

3 participants