Skip to content

fix(kanban): harden worker runtime ownership and preflight - #63637

Open
stevenbaert wants to merge 5 commits into
NousResearch:mainfrom
All-About-AI-YouTube:issue-424-worker-runtime-contract-v2-pr
Open

stevenbaert wants to merge 5 commits into
NousResearch:mainfrom
All-About-AI-YouTube:issue-424-worker-runtime-contract-v2-pr

Conversation

@stevenbaert

Copy link
Copy Markdown

Summary

This hardens the Kanban worker lifecycle so stale process IDs, reused PIDs, gateway restarts, and inherited provider routes cannot corrupt worker outcomes or terminate an unrelated process.

What changes

  • Bind worker exit evidence to a specific task, run, process identity, and process group.
  • Require current in-memory ownership before timeout/reclaim termination; otherwise fail closed.
  • Preserve bounded, redacted crash context for real process exits.
  • Validate workspace, profile, provider, model, credential reference, toolsets, and skills before spawning.
  • Persist the effective provider/model route and deterministically inherit it for reviewer/child tasks.
  • Prevent stale exit evidence from being consumed by a newer run.
  • Add regression coverage for PID reuse, gateway restart, ownership checks, route inheritance, invalid models/credentials, salvage, and POSIX/Windows termination paths.

Verification

Rebased onto current NousResearch/hermes-agent:main and run from a clean worktree:

scripts/run_tests.sh \
  tests/hermes_cli/test_kanban_worker_runtime_contract.py \
  tests/hermes_cli/test_kanban_db.py \
  tests/hermes_cli/test_kanban_core_functionality.py \
  tests/plugins/test_kanban_worker_runs.py -q

436 passed, 0 failed
python -m py_compile hermes_cli/kanban_db.py  # passed
git diff --check origin/main...HEAD          # passed

Independent exact-head QA also reproduced the two prior PID-reuse races and validated the canonical provider/model/auth and parent→reviewer inheritance paths.

Risk and scope

The changes are limited to Kanban worker lifecycle/preflight behavior and its tests. No gateway configuration, secrets, deployment, or runtime activation is included.

@alt-glitch alt-glitch added type/bug Something isn't working comp/cron Cron scheduler and job management P3 Low — cosmetic, nice to have labels Jul 13, 2026
@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for the detailed worker-ownership work. The current PR needs rework before its lifecycle changes can be salvaged.

Problems

  • hermes_cli/kanban_db.py:6953-6978 restores immediate blocking for clean-exit protocol violations. Current main deliberately changed this to a bounded, violation-only retry streak in 452861fdc (hermes_cli/kanban_db.py:6821-6886), including per-task retry precedence.
  • The ownership fail-closed rule is incomplete. The PR's _terminate_reclaimed_worker returns unverified ownership when its registry record is absent (hermes_cli/kanban_db.py:6313-6342), but release_stale_claims still releases the claim at :3715-3744; detect_stale_running has the same termination-attempt-only guard. An unverified live pre-restart worker can be overlapped by a respawn.

Suggested changes

  • Carry the registry/process-group work onto current main's bounded protocol-violation path.
  • Defer automatic stale reclaims when host-local ownership is indeterminate, and add restart-path coverage for TTL and heartbeat-stale reclaim.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 16, 2026

This branch has not been deployed

No deployments
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 P3 Low — cosmetic, nice to have 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-platform-windows Sweeper risk: may break or behave differently on native Windows sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants