Skip to content

fix(cron): scope-wrap oneshot-unit dispatch workers instead of spawning them doomed - #113624

Closed
kvnloo wants to merge 1 commit into
NousResearch:mainfrom
kvnloo:fix/113612-oneshot-scope-dispatch
Closed

kvnloo wants to merge 1 commit into
NousResearch:mainfrom
kvnloo:fix/113612-oneshot-scope-dispatch

Conversation

@kvnloo

@kvnloo kvnloo commented Sep 17, 2026

Copy link
Copy Markdown

What does this PR do?

Kanban workers dispatched from a dispatcher running under a Type=oneshot systemd unit were spawned as direct children of the unit's cgroup and SIGKILLed at service exit, with zero log output (Python's stdout is block-buffered to a file, so the SIGKILL before the ~4–8 KiB flush leaves the log empty). The board then loops spawn → kill → reclaim forever.

restart_safe_gateway_child_argv early-returned in_process for every non-gateway topology, silently ignoring require_restart_safe_scope=True. Now a non-gateway dispatcher with INVOCATION_ID set falls through to the same systemd-run --user --scope wrap the gateway path uses (and fails loud via the existing _degrade raise when no user scope can be created). A plain terminal dispatcher (no INVOCATION_ID) keeps the in-process path, and require_restart_safe_scope=False callers (cron's default) are unchanged.

Related Issue

Fixes #113612

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • tools/process_registry.py: restart_safe_gateway_child_argv now routes non-gateway systemd-unit dispatchers through the restart-safe scope wrap when require_restart_safe_scope=True; terminal dispatchers keep in_process.
  • tests/cron/test_restart_safe_worker.py: 4 new invariant tests (oneshot scope wrap, loud failure without a bus, terminal dispatcher unchanged, require=False unchanged).

How to Test

  1. On base, with INVOCATION_ID set and _is_supervised_gateway_process() False: restart_safe_gateway_child_argv(..., require_restart_safe_scope=True) returns in_process (silent passthrough — the bug).
  2. On this branch the same call either returns a systemd-run --scope-wrapped scoped dispatch (user bus reachable) or raises RuntimeError: cannot create restart-safe systemd scope… with the linger remedy.
  3. scripts/run_tests.sh tests/cron/test_restart_safe_worker.py — 26 passed, 1 skipped (pre-existing live-host guard); the 2 new behavior tests fail on base (red-on-base proven).
  4. Adjacent kanban dispatch suites green (test_kanban_cli_dispatch_passthrough, test_kanban_dispatch_claim_allowlist, test_kanban_dispatch_lock, test_kanban_dispatch_tick_hook).

Checklist

Code

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A (docstring updated in the helper)
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A (Linux-only path, guarded by _IS_LINUX)

authored with AI assistance (Muse, Meta's Muse Spark) under the contributor's direction; reviewed and approved by the contributor before submission.

…ng them doomed

Kanban workers dispatched from a dispatcher running under a Type=oneshot
systemd unit were spawned as direct children of the unit's cgroup and
SIGKILLed at service exit with zero log output (NousResearch#113612). The helper's
early-return treated every non-gateway topology as safe in-process, silently
ignoring require_restart_safe_scope=True. Now a non-gateway dispatcher with
INVOCATION_ID set falls through to the same systemd-run --scope wrap (and
fails loud when no scope can be created); terminal dispatchers keep the
in-process path.

authored with AI assistance (Muse, Meta's Muse Spark) under the contributor's direction; reviewed and approved by the contributor before submission.
@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/cron Cron scheduler and job management sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Sep 17, 2026
@whyyagswhy

Copy link
Copy Markdown

Independent verification on the PR head (b669d32): tests/cron/test_restart_safe_worker.py passes locally, 27/27 on Linux (canonical runner).

The condition split is exact: no INVOCATION_ID keeps the in-process path, systemd-managed non-gateway without the strict requirement keeps it too, and require_restart_safe_scope under a oneshot unit now scope-wraps instead of spawning children into a dying cgroup. Fails loud when no scope can be created. No findings.

@kyssta-exe

Copy link
Copy Markdown

Summary

Dispatchers running under a non-gateway systemd unit (Type=oneshot, INVOCATION_ID set) no longer spawn workers as direct cgroup children that get SIGKILLed at unit exit. With require_restart_safe_scope=True they now take the same systemd-run --user --scope wrap as the gateway path, or raise loudly when no scope is available. Fixes #113612.

What changed

  • tools/process_registry.py: restart_safe_gateway_child_argv splits the old early-return: no INVOCATION_ID keeps in_process; non-gateway + require=True falls through to the scope wrap / loud raise; require=False (cron default) unchanged.
  • tests/cron/test_restart_safe_worker.py: 4 invariant tests (oneshot wrap, loud failure without bus, terminal unchanged, require=False unchanged).

Strengths

  • The silent-passthrough bug is fixed in the right direction (fail loud, not fail doomed) with the existing linger remedy reused.
  • Tests pin all four quadrants of the INVOCATION_ID x require matrix, including proving the systemd probe does not run where it shouldn't.

Findings

  • Rollout note, not a blocker: existing systemd-unit dispatchers without a user bus/linger will now raise instead of (doomed) spawning — intended, but operators should expect the new loud error until lingering is enabled.
  • No CI checks reported on the branch; the focused suite plus adjacent kanban dispatch suites are reported green in the PR body.

Verdict

Looks good to merge

Reviewed using Hermes-Agent

teknium1 added a commit that referenced this pull request Sep 18, 2026
…spatch scope-wraps workers

Two defects at the same spawn boundary (hermes_cli/kanban_db_dispatch.py::
_restart_safe_worker_argv -> tools/process_registry.py::restart_safe_gateway_child_argv).

#114720 — a managed-gateway dispatcher whose user D-Bus session had gone away
raised the restart-safe-scope RuntimeError at spawn; the ordinary spawn
except-clause fed it to _record_task_failure, which advanced
consecutive_failures and at failure_limit parked the card as a bare `blocked`
(block_kind NULL) with a `gave_up` run. Nothing about the card had run.

- restart_safe_gateway_child_argv raises RestartSafeScopeUnavailable
  (RuntimeError subclass) so the dispatcher can tell a host refusal from a
  card failure.
- _record_task_failure(infrastructure=True): run + event carry
  `infrastructure: true`, consecutive_failures is left alone, the breaker
  never trips; the card stays `ready` with the linger remedy in
  last_failure_error and a warning in the dispatcher log.
- check_respawn_guard returns `infrastructure_cooldown` while the latest run
  is such a refusal inside the rate-limit cooldown window, so a dead bus is
  retried spaced instead of every tick.
- require_restart_safe_scope=True keeps its hard-fail semantics under the
  managed gateway.

#113612 — `hermes kanban dispatch` from an operator's Type=oneshot systemd unit
spawned unmanaged workers: the gate `_is_supervised_gateway_process()` is
False for a CLI dispatcher, so the helper returned the bare argv and the
workers died at the unit's cgroup teardown with empty logs, while the
`running` rows waited for reclaim.

- New outlives_parent=True (kanban only; cron blocks its caller and keeps the
  old gate): under any systemd unit (INVOCATION_ID) a fire-and-forget worker
  is scope-wrapped when the user bus is reachable. Without a bus it degrades
  with a loud once-per-process warning naming the consequence and both
  remedies (enable-linger, KillMode=process) rather than refusing — the
  unit's KillMode/lifetime is unknowable and a long-lived sequencer without
  linger must keep working.

Docs: kanban.md "Workers and systemd cgroups" + guard reasons; cron.md note.

Based on analysis from PR #113624.
Co-authored-by: Kevin Rajan <7121943+kvnloo@users.noreply.github.com>
@teknium1

Copy link
Copy Markdown
Collaborator

Thanks @kvnloo. Your change was salvaged into #114886 with your authorship preserved (co-authored); #114886 — fix(kanban): host spawn refusals no longer park cards as blocked; oneshot-unit dispatch workers survive unit exit (#114720, #113612, salvage #113624) — is now merged on main at eb28bc1bf7e9, closing issues #114720, #113612. 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.

@teknium1 teknium1 closed this Sep 18, 2026
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:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

5 participants