Skip to content

fix(kanban): stop workers after lifecycle handoff - #85408

Open
ataraxiaone wants to merge 1 commit into
NousResearch:mainfrom
ataraxiaone:ranger/kanban-gap-closure-upstream-20260813
Open

fix(kanban): stop workers after lifecycle handoff#85408
ataraxiaone wants to merge 1 commit into
NousResearch:mainfrom
ataraxiaone:ranger/kanban-gap-closure-upstream-20260813

Conversation

@ataraxiaone

Copy link
Copy Markdown

What does this PR do?

Stops a Kanban worker immediately after it successfully hands custody off through complete, block, request-review, or request-changes, and binds the actual runtime provider/model/API mode/session to the corresponding durable run receipt.

This closes two concrete failure modes observed in a bounded native-Kanban campaign:

  • a stale worker continued writing after its lifecycle handoff; and
  • task receipts could describe requested profile configuration without proving the runtime route that actually executed the worker.

The lifecycle latch is deliberately narrow: it requires both a real HERMES_KANBAN_TASK and a dispatcher-owned worker context. Ordinary orchestrators, delegated children, inherited cron contexts, and failed lifecycle calls do not stop.

Related Issue

Related to #82591. This is a bounded lifecycle/receipt hardening slice, not closure of that epic.

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • Propagate trusted runtime identity outside model-controlled tool arguments.
  • Stamp provider, model, API mode, session, and source on all four custody-ending Kanban lifecycle paths.
  • Persist metadata for blocked and changes-requested runs.
  • Latch only successful dispatcher-owned worker handoffs.
  • Cancel later same-batch calls, including calls in later execution segments.
  • End the worker conversation loop without another provider iteration.
  • Cover spoof resistance, all four lifecycle outcomes, context boundaries, failed calls, same-segment cancellation, and later-segment cancellation.

How to Test

  1. scripts/run_tests.sh $(python3 -c 'import glob; print(" ".join(sorted(glob.glob("tests/**/*kanban*.py", recursive=True))))')
  2. scripts/run_tests.sh tests/run_agent/test_run_agent.py::TestConcurrentToolExecution::test_invoke_tool_dispatches_to_handle_function_call tests/tools/test_kanban_runtime_receipts.py tests/run_agent/test_tool_batch_segmentation.py
  3. python3 -m compileall -q agent hermes_cli model_tools.py tools/kanban_tools.py
  4. uv run --with ruff ruff check agent/agent_init.py agent/agent_runtime_helpers.py agent/conversation_loop.py agent/tool_executor.py hermes_cli/kanban_db.py model_tools.py tools/kanban_tools.py tests/run_agent/test_run_agent.py tests/run_agent/test_tool_batch_segmentation.py tests/tools/test_kanban_runtime_receipts.py
  5. git diff --check origin/main...HEAD

Local results on macOS 15.7.8 / Python 3.11.15:

  • Kanban suite: 396 passed, 2 skipped
  • Targeted lifecycle/receipt suite: 40 passed, 1 skipped
  • Adjacent dispatch contract: passed
  • Ruff, compileall, and diff checks: passed

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 repository's canonical focused suites
  • I've added tests for my changes
  • I've tested on macOS 15.7.8

Documentation & Housekeeping

  • Documentation updates are N/A; behavior and trust boundary are covered by code comments and tests
  • cli-config.yaml.example update is N/A; no config keys changed
  • CONTRIBUTING.md / AGENTS.md update is N/A
  • Cross-platform impact considered; Windows-specific segmented-dispatch coverage remains in CI
  • Tool schema changes are N/A; trusted runtime identity is internal middleware data, not a model-visible argument

Screenshots / Logs

No UI change. The PR is covered by canonical tests and an independently reviewed live-campaign evidence packet. No credentials, external source-system writes, or production mutations are part of this patch.

@ataraxiaone

Copy link
Copy Markdown
Author

@kshitijk4poor Ready for maintainer review when convenient. GitHub is holding the first-time-contributor CI runs for approval. The focused Kanban suite passed locally (396 passed, 2 skipped), the lifecycle/receipt suite passed (40 passed, 1 skipped), and the exact head is e8ffec8.

@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/cron Cron scheduler and job management comp/tools Tool registry, model_tools, toolsets labels Aug 13, 2026
@enzo-adami

Copy link
Copy Markdown
Contributor

Focused independent validation on the PR HEAD (e8ffec887a):

scripts/run_tests.sh \
  tests/tools/test_kanban_runtime_receipts.py \
  tests/run_agent/test_tool_batch_segmentation.py \
  tests/run_agent/test_run_agent.py -q

284 passed, 0 failed (1 Windows-only skip on macOS)

I also checked the handoff ordering: the lifecycle calls are sequential barriers, the successful result is durably flushed before later segments are cancelled, and the conversation loop exits before another model turn. I did not find a counterexample in that scoped contract.

This is not yet a current-main validation: the branch has a real content conflict in agent/tool_executor.py against origin/main 165c889e5b... (and consequently against #86609/#86610 once rebased). Because that file's execution/persistence logic has moved materially, this should be reconciled on fresh main and the same focused suite rerun before treating the green result above as merge evidence.

@enzo-adami

Copy link
Copy Markdown
Contributor

I found and reproduced two ownership races that are not covered by the current 284-test suite:

  1. Each lifecycle handler commits its guarded transition and then calls latest_run() to construct the tool result. If the dispatcher claims a successor between those operations, the old worker receives the successor run ID. A deterministic kanban_block oracle monkeypatching that post-commit lookup returns run N+1 on the current PR HEAD.
  2. _record_successful_kanban_handoff() accepts any {ok: true} lifecycle payload, and both consumers stop on a truthy latch. A payload for a different task/run can therefore latch, while a latch from run N can stop run N+1.

I prepared a narrow extension on top of the author's exact HEAD, preserving the original commit and targeting the author's branch: ataraxiaone#1

The extension captures the guarded run before transition, requires exact dispatcher task+run equality before latching, and revalidates/clears the latch at both consumption points. Added deterministic regressions cover wrong-task, wrong-run, missing-run, post-commit successor, stale-latch successor, and effective-runtime-vs-requested-config provenance.

Validation: focused suite 48 passed, 1 skipped; Ruff and git diff --check pass. The broader suite reached 291 passed, 1 skipped; its sole failure is environmental and unrelated (anthropic is absent from the borrowed venv for TestAnthropicInterruptHandler). The original PR suite had already passed 284 passed, 1 skipped on the unmodified HEAD.

@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference, author can ignore or act on any point.

fix(kanban): stop workers after lifecycle handoff

Good, well-tested design: trusted runtime identity stamped over model-supplied metadata, the latch + cancelled-result plumbing across segments, and the dispatcher-owned context gate.

  • runtime_identity is constructed in three places (agent/agent_runtime_helpers.py, agent/tool_executor.py::_runtime_identity, plus the test fixture) — extract a single helper to prevent drift in the key set or the source value.
  • Inconsistency in failure handling: the segmented path checks _flush_session_db_after_tool_progress(...) and returns on False, while the non-segmented path (agent/tool_executor.py, the handoff_succeeded block) calls it without checking the return before breaking. Align the two failure modes.
  • runtime_identity["session_id"] comes from the agent object, while _stamp_worker_session_metadata separately stamps worker_session_id from the HERMES_SESSION_ID env var. If those ever disagree (child-agent contexts, session handoff), run metadata records two different session ids — confirm the intent and document which one is authoritative for forensics.
  • The latch keys on payload.get("ok") is True against the tool-result JSON envelope; if a future kanban tool changes its result shape, the latch silently stops firing (worker keeps running after a successful handoff — fail-open). The _KANBAN_LIFECYCLE_HANDOFF_TOOLS allow-list plus the env/context gates mitigate this; a comment noting the envelope dependency would help future maintainers.

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 comp/tools Tool registry, model_tools, toolsets P3 Low — cosmetic, nice to have type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants