fix(kanban): prevent nested hermes chat from inheriting parent Kanban ownership (#70809) - #70898
fix(kanban): prevent nested hermes chat from inheriting parent Kanban ownership (#70809)#70898webtecnica wants to merge 2 commits into
Conversation
… ownership (NousResearch#70809) Derive Kanban worker ownership once at the CLI boundary by matching the dispatcher query marker ("work kanban task <id>") to HERMES_KANBAN_TASK and store the result in a ContextVar. Lifecycle code (heartbeat, goal loop, tool gates, signal handler, stop-nudge, turn-finalizer) now reads the verified ContextVar instead of re-reading os.environ, so a nested hermes chat subprocess that inherited HERMES_KANBAN_* env vars is never treated as the parent worker. The fix spans 12 files and adds is_kanban_worker_owner() / set_kanban_worker_owner() to agent/delegation_context.py, then replaces os.environ.get("HERMES_KANBAN_TASK") checks in: - cli.py (CLI boundary, signal handler, goal loop, exit code) - run_agent.py (_touch_activity heartbeat) - agent/turn_finalizer.py (iteration budget timeout) - agent/kanban_stop.py (stop-nudge guard) - model_tools.py (tool cache key, kanban toolset auto-inject) - tools/kanban_tools.py (tool gates, task-id resolution, ownership enforcement, heartbeat) - tools/send_message_tool.py (send_message gate) - agent/skill_utils.py (skill visibility filter) - hermes_cli/doctor.py (runtime-gated diagnostics) - hermes_cli/kanban.py (_worker_run_id_for) Each replacement falls back to the env var on import failure.
… ownership (NousResearch#70809) Add missing cli.py ContextVar gates for signal handler, goal loop, and exit-code logic. Fix conversation_loop.py logger to not read env var. Add tests for ContextVar-based ownership verification.
|
Read-only review of current head One remaining coverage gap: the new tests simulate the ownership gate, but do not exercise a real nested This would make the #70809 process-boundary contract explicit without requiring a production database or live dispatcher. |
|
Thanks for tracing the ownership problem to inherited process environment and introducing a process-local ownership signal. The current main premise is real: Problems
Suggested changes
Automated hermes-sweeper review. |
SummaryOne open PR, #70898, addresses #70809 by deriving Kanban worker ownership from an exact dispatcher-query/task match and propagating that process-local identity across lifecycle consumers. The diff blocks several nested-CLI ownership paths, but environment-only classification and runtime gating remain outside the conversion. Related pull requests
Suggested consolidationKeep #70898 open with a salvage path, consistent with the maintainer-bot keep_open review: retain the CLI-boundary ownership ContextVar, convert the remaining ownership-sensitive environment checks, remove fail-open environment fallbacks from worker-only lifecycle paths, and add a bounded inherited-environment full-CLI regression with matching and non-matching queries. It is the sole and best available implementation for #70809, so there are no duplicate PRs to close. Complex graphflowchart LR
classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
classDef best stroke-width:3px,stroke:#b45309
classDef target stroke-width:3px,stroke:#4338ca
I70809(["issue #70809 (open)"])
P70898["PR #70898 (open)"]
P70898 -->|best fix| I70809
class I70809 open
class P70898 open
class P70898 best
class P70898 target
click I70809 "https://github.com/NousResearch/hermes-agent/issues/70809"
click P70898 "https://github.com/NousResearch/hermes-agent/pull/70898"
Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label). Cross-PR triage: Reviewed 1 pull request and 1 issue in this complex. Each diff was read against this issue; Assessment working set: 31 kB of PR diffs, 4 kB of issue/PR text, <1 kB of discussion (1 comments), 3 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch. |
Steering note: narrower alternative fix proposed in #81843Thanks for tracing the nested-CLI ownership leak to inherited While this PR has been waiting on those requested changes, a narrower fix for the same root cause was proposed in #81843: it strips If maintainers prefer this PR's ContextVar approach as the deeper fix, #81843 composes with it rather than conflicting — it just narrows what a nested process can see. Otherwise, closing this PR in favor of #81843 is a reasonable option. Please let us know which direction you'd like to take, or if you have questions about how the two changes interact. |
Problem
A dispatcher-owned Kanban worker can run a nested
hermes chat -q ...via terminal. That nested CLI inheritsHERMES_KANBAN_TASKenv vars and is treated as the parent worker itself — can load the parent card, emit heartbeats, and finalize the parent run.Root cause
Several lifecycle paths use
os.environ.get('HERMES_KANBAN_TASK')as proof of ownership. Environment presence cannot distinguish the dispatcher-owned entry process from a nested CLI subprocess.Fix
agent/delegation_context.py— Added_KANBAN_WORKER_OWNERContextVar withset_kanban_worker_owner()/is_kanban_worker_owner(). Also addedscrub_kanban_env()anddelegated_child_subprocess_env()for env propagation safety. (Previously committed.)cli.py— Three raw env reads converted to ContextVar-gated checks:os._exit(0)path)_run_kanban_goal_loop_q(goal mode worker loop)agent/conversation_loop.py— Logger no longer readsHERMES_KANBAN_TASKfrom env.Consumer code with ContextVar-first patterns (previously committed):
agent/kanban_stop.py,agent/turn_finalizer.py,run_agent.py,agent/skill_utils.py,model_tools.py,tools/send_message_tool.py,tools/kanban_tools.py,hermes_cli/kanban.py,hermes_cli/doctor.pyTests — 4 new tests for ContextVar-based ownership verification.
Closes #70809