Skip to content

fix(kanban): a delegated child must not heartbeat the parent worker's card - #113314

Closed
Finn763 wants to merge 1 commit into
NousResearch:mainfrom
Finn763:fix/105202-descr
Closed

Finn763 wants to merge 1 commit into
NousResearch:mainfrom
Finn763:fix/105202-descr

Conversation

@Finn763

@Finn763 Finn763 commented Sep 16, 2026

Copy link
Copy Markdown

What the issue reports, and what is actually still true at HEAD

#105202 says a delegated subagent can complete its parent's Kanban card (closing it and deleting the workspace). Checked before touching anything: the complete/close path does not reproduce on this base — upstream already carries the #69837 machinery the issue's comments point at (a delegated-child context, kanban in the child toolset block list, _reject_delegated_child_mutation, _default_task_id failing closed). The new test for the real delegate path (parent claimed, HERMES_KANBAN_TASK set, child dispatches kanban_complete with no task id) is green at base too, and I have left it that way instead of manufacturing a red.

The residual is the same class on a non-terminal path, and no test covered it. tools/kanban_tools.py:444 heartbeat_current_worker_from_env read HERMES_KANBAN_TASK straight out of the environment with no ownership predicate — the only mutating consumer of that variable that was not owner-gated (its sibling inject_new_comments_from_env forty lines below already was, per #112817). A delegate child shares the parent's process and inherits the task id and the claim lock, so it could:

  • extend the parent's tasks.claim_expires / task_runs.claim_expires while the parent was busy, and
  • consume the single shared _AUTO_HEARTBEAT_MIN_INTERVAL_SECONDS slot, potentially suppressing the worker's own next auto-heartbeat.

Fix (one line of semantics, mirroring the sibling):

tid = os.environ.get("HERMES_KANBAN_TASK") if _is_dispatcher_owned_worker() else None

Rejected alternatives: clearing/unsetting os.environ for the child (process-global — it would break the worker's own claim heartbeat and the gateway watchers, and the issue itself says not to); adding kanban_complete to DELEGATE_BLOCKED_TOOLS (a no-op — that list only strips whole toolsets).

Evidence

RED → GREEN (my own mutation): removing the ownership predicate restores the reported class of bug and fails

assert kanban_tools.heartbeat_current_worker_from_env() is False
E   AssertionError: assert True is False        tests/tools/test_delegate_child_parent_card_scope.py:139

Restored → 5/5 on the new file (7 passed with the descendant-scope neighbour).

Protection surface, all green: the parent worker still completes its own card; the owner's own heartbeat tick still returns True and advances claim_expires while the child does not consume the shared 60 s slot; ordinary delegation outside a Kanban context is unchanged; and the child inherits neither the Kanban identity nor the toolset (_resolve_child_toolsets drops kanban and adds it to disabled_toolsets).

Consumer inventory (every non-test reader of HERMES_KANBAN_TASK, with the safety verdict) is in the commit body: one ungated mutating consumer existed — the auto-heartbeat fixed here; inject_new_comments_from_env was already gated; _default_task_id/_own_task_env/_enforce_worker_task_ownership/_require_orchestrator_tool/_visible/kanban_create are gated by the #69837 layer; hermes_cli/kanban.py and the MCP handoff tools only ever see the var through a subprocess, where delegated_child_subprocess_env() scrubs HERMES_KANBAN_* and the DB is fenced read-only; the rest are read-only/display or worker-process paths. Verdict: the env is isolated by context, not by mutation.

One pre-existing red in the neighbour sweep is unrelated and proven by name: test_kanban_descendant_scope.py::test_terminal_descendants_cannot_mutate_even_after_task_is_removed (StopIteration, no SCOPE_RESULT — Windows subprocess) fails identically on origin/main with the same case name; I reproduced that myself.

Refs #105202 (not Fixes: the issue's headline path is already closed upstream; this is the remaining leak).

Not done

  • Not fixed, disclosed: the pre-existing Windows-subprocess red above.
  • The same widening exists for in-process cron jobs (they inherit the var too); cron has its own claim heartbeats and an in-process cron run blocks the worker like a long tool call, so no new starvation class is introduced — the cron isolation suite stays green, but this was not independently test-covered here.

…s card

Auto-heartbeat (agent._touch_activity -> heartbeat_current_worker_from_env)
read HERMES_KANBAN_TASK straight from the process env. A delegate_task child
shares the worker's process, so every child activity tick extended the PARENT's
claim_expires (tasks + task_runs) using the inherited task id and claim lock,
and — the timer being one shared process-global — could also suppress the
worker's own next heartbeat.

Gate the read on is_dispatcher_owned_worker_context(), exactly like the sibling
inject_new_comments_from_env. The owner's own ticks are untouched, and the
parent's claim stays alive during a child run through delegate_tool's own
activity heartbeat (_Heartbeat.tick -> parent_agent._touch_activity).

tests/tools/test_delegate_child_parent_card_scope.py pins the whole class: a
child's bare kanban_complete() cannot close the parent card (card stays running,
run stays running, workspace survives), it cannot extend the parent's claim, and
neither the worker's own kanban_complete() nor a delegation outside any Kanban
context changes behaviour.
@teknium1

Copy link
Copy Markdown
Collaborator

Thanks @Finn763. This was resolved by #114793, which covers the same fix; #114793 — fix(kanban): fenced worker auto-heartbeat reports failure and warns instead of silently returning True (#113609, salvage #113616) — is now merged on main at e3e086689973, closing issues #113609. Closing this PR in favour of the landed change; if you see a case it does not cover, please open a fresh issue with the repro and tag it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cron Cron scheduler and job management comp/tools Tool registry, model_tools, toolsets P3 Low — cosmetic, nice to have 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