Repository navigation
fix(gateway): skip restart-safe scoped cron runs in the restart wait, keyed per profile - #131071
Merged
unsupportedpastels merged 2 commits intoOct 2, 2026
Conversation
…estart wait A cron run handed to an external worker in its own transient systemd scope outlives the gateway: the shutdown drain deliberately leaves it unmarked (``mark_running_jobs_interrupted``) and its final send rides the durable delivery queue, drained by whichever gateway is live next. The after-turn restart wait still counted it as active work, so ``/restart``, SIGUSR1 and ``hermes update`` held the gateway in ``draining`` — refusing every new turn — for up to ``agent.restart_after_turn_timeout`` (30 min by default) while the worker kept running regardless. Observed in the field: a 100-minute job turned a restart into a ~30-minute lockout. - ``cron.scheduler`` records whether an accepted handoff got its own scope (``dispatch.mode == "scoped"``) and exposes ``get_restart_safe_external_job_ids()``; runs already reported as wedged are excluded there too, because the gateway subtracts both counts and a run in both sets would be subtracted twice. - ``_awaitable_work_count()`` subtracts that count as a second, disjoint exclusion; ``_describe_active_work()`` marks the unit (``restart_safe``). - A degraded worker (external, but no reachable user bus, so it shares the gateway cgroup) is NOT listed and is still awaited — the restart would kill it mid-run. - The wait and observation log lines now name both exclusion classes instead of claiming everything still listed will be interrupted. Tests: tests/gateway/test_restart_drain.py (skip + control), tests/cron/test_restart_safe_worker.py (registry contract). Docs: updating.md restart section, cli-config.yaml.example. (cherry picked from commit 6a2f0b7)
A scoped or wedged daily-brief in profile A must not hide a degraded or live daily-brief in profile B. Split the scheduler's in-flight keys before counting and add awaitable cron runs independently of non-cron work. Carry the per-run restart_safe flag to the observer and preserve the existing bare-ID accessors for unrelated callers. Thanks to @ehz0ah for diagnosing the multiplexed collision and proposing the profile-keyed accounting and mixed scoped/degraded regression case. Preserve @Superfly73's source commit separately; map their verified bare noreply email for the contributor CI gate. Refs NousResearch#130987 Co-authored-by: z1gon <130889443+ehz0ah@users.noreply.github.com>
unsupportedpastels
left a comment
Author
There was a problem hiding this comment.
Thanks @Superfly73 for the restart-safe scoped-worker fix and @ehz0ah for identifying the cross-profile collision. The source commit is preserved with cherry-pick provenance; the follow-up keeps the exclusions keyed by profile and credits the review diagnosis. Local cron/gateway suites and the main/source/salvage behavioral probe are documented in the PR. Merge only after required exact-head checks and fresh independent review pass.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Fixes #130987.
Healthy cron runs already executing in an isolated systemd scope can hold the restart after-turn wait for the full cap even though they survive the gateway restart.
Fix and prior work
Preserves @Superfly73's #130988 commit via cherry-pick -x: records successful scoped handoffs, cleans up their registry, excludes them from the wait and updates the docs. Thank you for the report, scoped/degraded distinction and implementation.
Adds the profile-keyed accounting requested by @ehz0ah in review: two profiles' same-ID runs remain distinct, so a scoped or wedged run cannot hide a degraded/live sibling. The diagnosis and regression direction are credited by co-author trailer. The bare-ID accessor introduced by the source PR is replaced with disjoint keyed counts; unrelated existing bare-ID APIs remain unchanged. The observer gets the per-run restart_safe flag.
Verification
Canonical runner: tests/cron + tests/gateway: 10,520 passed, 42 skipped; main baseline: 10,515 passed, 42 skipped. No failures. The two same-ID behavioral regressions fail with source PR production restored (0 awaitable instead of 1). Real-import temporary-profile A→B→A probe reproduces the original wait on main and the collision on #130988, and passes on this branch. Ruff, Windows-footgun scan and attribution audit pass.
No live systemd restart or native Windows/macOS run was performed. Hosted CI must pass before merge.
Scope / risk
Only restart wait accounting and scoped-worker bookkeeping; no timeout/config-key, subprocess launch, interruption or durable-delivery policy changes. Degraded workers remain awaited. Not in scope: embedded Kanban drain (#103961), stop-phase timeout budget (#129957).