Skip to content

fix(cli): scope async delegation delivery to session - #64240

Closed
rabadaki wants to merge 1 commit into
NousResearch:mainfrom
rabadaki:fix/cli-async-delegation-session-routing
Closed

fix(cli): scope async delegation delivery to session#64240
rabadaki wants to merge 1 commit into
NousResearch:mainfrom
rabadaki:fix/cli-async-delegation-session-routing

Conversation

@rabadaki

Copy link
Copy Markdown
Contributor

Summary

  • Scope classic CLI async-delegation completion drains to the visible session.
  • Preserve delivery across context-compression session rotation by resolving the dispatch key to its active continuation.
  • Use one shared drain helper for both idle and post-turn notification paths.
  • Add focused regression coverage for session identity, foreign-session rejection, and compression lineage.

Problem

#60869 added session-aware async completion filtering to the process registry and routed gateway/TUI callers, but the classic CLI retained two unsafe call sites:

  • The idle path derived its session key from turn-local context after that context could be reset.
  • The post-turn path drained without a session filter.

Because the process notification queue is profile-global, a sibling classic CLI process could claim and acknowledge a completion dispatched by another session. The origin session would then never receive it.

This is a focused follow-up to #58684 and #60869. Non-delegation process and watch notifications retain their existing behavior; ownership filtering applies only to async_delegation events in the registry.

Verification

venv/bin/ruff check cli.py tests/cli/test_cli_async_delegation_delivery.py
All checks passed

venv/bin/python -m pytest -q -o addopts= \
  tests/cli/test_cli_async_delegation_delivery.py \
  tests/tools/test_process_registry.py \
  tests/gateway/test_async_delegation_session_binding.py \
  tests/tui_gateway/test_delegation_session_lifecycle.py
124 passed

Manual regression on macOS:

  1. Started two classic CLI processes using one disposable profile.
  2. The origin process dispatched a delayed delegation and remained active.
  3. A fresh sibling process started after completion persistence and restored pending events.
  4. The sibling did not consume the foreign completion.
  5. The origin received the completion exactly once; durable delivery attempts remained 1 and the recorded owner PID matched the origin process.

The repository-wide wrapper was also sampled but is not a clean macOS gate in this checkout because unrelated Linux-only tests require systemctl. The focused CLI/process/session suites above are clean.

Tested on macOS 26.3.1 with Python 3.11.

@rabadaki
rabadaki marked this pull request as ready for review July 14, 2026 06:57
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/cli CLI entry point, hermes_cli/, setup wizard tool/delegate Subagent delegation sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Jul 14, 2026
teknium1 added a commit that referenced this pull request Jul 15, 2026
…atch identity

Three-layer companion to the salvaged CLI drain-ownership fix (#64240):

1. restore_undelivered_completions stamps restored=True (in-memory only)
   on every durable completion re-enqueued at process start.
2. drain_notifications' legacy unfiltered branch re-queues restored
   events instead of consuming them — a fresh process can no longer
   adopt a dead session's delegation results (#64484). Same-process
   keyless events keep the legacy behavior.
3. delegate_tool's async dispatch now falls back to the parent agent's
   durable session_id when the approval-context key resolves empty (the
   CLI case), so the CLI's new positive-ownership drain can actually
   claim its own completions instead of failing closed on ''.
@teknium1 teknium1 closed this Jul 15, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Merged via #64593 — your commit cherry-picked onto current main with authorship preserved (rebase-merge). Two companion layers added on top: (1) restore_undelivered_completions stamps restored=True and the legacy unfiltered drain fails closed on those events, so every future bare drain_notifications() caller is safe, not just the CLI; (2) delegate_tool's async dispatch now stamps the parent agent's durable session_id when the approval-context key is empty — without that, your (correct!) positive-ownership filter would have blocked the CLI from claiming its own completions, since CLI dispatches carried session_key="". Live cross-process E2E before merge: a dead session's completion → fresh CLI session receives nothing on both drain sites; the owner's --resume still gets it. Thanks for the clean ownership + lineage-resolution design.

Gravezzz pushed a commit to Gravezzz/hermes-agent that referenced this pull request Jul 21, 2026
…atch identity

Three-layer companion to the salvaged CLI drain-ownership fix (NousResearch#64240):

1. restore_undelivered_completions stamps restored=True (in-memory only)
   on every durable completion re-enqueued at process start.
2. drain_notifications' legacy unfiltered branch re-queues restored
   events instead of consuming them — a fresh process can no longer
   adopt a dead session's delegation results (NousResearch#64484). Same-process
   keyless events keep the legacy behavior.
3. delegate_tool's async dispatch now falls back to the parent agent's
   durable session_id when the approval-context key resolves empty (the
   CLI case), so the CLI's new positive-ownership drain can actually
   claim its own completions instead of failing closed on ''.
randlee pushed a commit to randlee/hermes-agent that referenced this pull request Aug 11, 2026
…atch identity

Three-layer companion to the salvaged CLI drain-ownership fix (NousResearch#64240):

1. restore_undelivered_completions stamps restored=True (in-memory only)
   on every durable completion re-enqueued at process start.
2. drain_notifications' legacy unfiltered branch re-queues restored
   events instead of consuming them — a fresh process can no longer
   adopt a dead session's delegation results (NousResearch#64484). Same-process
   keyless events keep the legacy behavior.
3. delegate_tool's async dispatch now falls back to the parent agent's
   durable session_id when the approval-context key resolves empty (the
   CLI case), so the CLI's new positive-ownership drain can actually
   claim its own completions instead of failing closed on ''.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard P2 Medium — degraded but workaround exists 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

Development

Successfully merging this pull request may close these issues.

3 participants