Skip to content

fix(process): negotiate the systemd properties a transient scope accepts - #102508

Closed
JoaoMarcos44 wants to merge 1 commit into
NousResearch:mainfrom
JoaoMarcos44:fix/systemd-scope-property-negotiation-102486
Closed

JoaoMarcos44 wants to merge 1 commit into
NousResearch:mainfrom
JoaoMarcos44:fix/systemd-scope-property-negotiation-102486

Conversation

@JoaoMarcos44

Copy link
Copy Markdown

Problem

On every systemd older than v253, each restart-safe worker dispatch fails:

Restart-safe cron worker dispatch failed: cannot create restart-safe systemd scope
for gateway child: systemd-run --user --scope is unavailable

_systemd_run_user_scope_available() probes the backend with the same property set
the worker spawn will use, and reads any non-zero exit as "no scope backend". One of
those properties, OOMPolicy=, became valid on scope units only in systemd
v253 ("Scope units now support OOMPolicy=", systemd v253 NEWS). Ubuntu 22.04
ships 249; Debian 12 and RHEL 9 ship 252. All of them answer the probe with
Unknown assignment: OOMPolicy=kill and exit 1 before creating anything.

The verdict is then cached, and restart_safe_gateway_child_argv() fails closed on
every dispatch that goes through it — cron/scheduler.py::_launch_external_cron_worker
and hermes_cli/kanban_db.py::_restart_safe_worker_argv. A hardening knob nobody
asked about becomes a total worker outage, and the operator sees only the verdict,
never the Unknown assignment line that caused it.

Root cause

The probe asserts a hardcoded property set, so one property this manager cannot
parse is indistinguishable from a missing user bus. The same list is then written a
second time in _build_systemd_scope_argv(), so the spawn can ask for something the
probe never proved.

Fix

Probe the desired set; when systemd names a property it cannot parse, drop that
property and probe again. The surviving set is cached and the scope builder spawns
with exactly it.

Host Before After
systemd 249 / 252 (Ubuntu 22.04, Debian 12, RHEL 9) every dispatch fails closed scope created with MemoryAccounting + MemoryMax; OOMPolicy dropped, once, with a warning
systemd >= 253 (Ubuntu 24.04, Fedora 38+) works unchanged — OOMPolicy=kill kept
no user D-Bus session fails closed fails closed, now quoting the systemd error

An unsupported knob costs that knob, not the isolation every dispatch depends on.
Failures that are not a property rejection are deliberately left alone: a missing
user bus is real unavailability, and retrying with a smaller set would only spin. That
path still fails closed, preserving the #101940 restart-durability contract untouched.

%%{init: {'theme': 'dark', 'themeVariables': { 'primaryColor': '#8b0000', 'mainBkg': '#0a0204', 'primaryTextColor': '#ffccd5', 'primaryBorderColor': '#ff0038', 'lineColor': '#ff0038'}}}%%
graph TD
    A[Cron / Kanban Dispatch] --> B[Scope Availability Probe]
    B --> C{systemd-run exit code}
    C -->|0| D[Cache Accepted Property Set]
    C -->|Unknown assignment: PROP| E[Drop Rejected Property]
    C -->|Other failure e.g. no user bus| F[Fail Closed + Quote systemd Error]
    E --> B
    D --> G[Build Scope With The Proven Set]
    G --> H[Restart-Safe Worker Running]
Loading

Relationship to open work

Supersedes #102357. That PR found the same rejected property on a live host — the
diagnosis is theirs and it is correct. It removes OOMPolicy=kill statically from both
argv sites, which also removes the OOM-kill semantics on every host where the property
is valid (systemd >= 253), leaves the next added property free to repeat the same
outage, and is currently red on Python tests / Run tests because
tests/tools/test_process_registry.py:1960 still pins OOMPolicy=kill. Its own review
thread asks for exactly this shape: "If per-worker OOM kill matters, consider re-adding
it conditionally behind a support probe later."
This PR is that probe. If maintainers
prefer the one-line removal, #102357 should land instead of this.

Not this PR's layer:

Test plan

tests/tools/test_process_registry.py (4 new, in the existing POSIX-only class):

  • test_probe_negotiates_away_a_rejected_scope_property — Unknown assignment: OOMPolicy=kill on the first probe, success on the retry; asserts the cached set is
    ("MemoryAccounting", "MemoryMax"), that the retry carries a fresh unit name, and
    that the memory bound survived.
  • test_probe_does_not_negotiate_a_missing_user_bus — Failed to connect to bus probes
    exactly once and stays unavailable.
  • test_scope_argv_uses_the_negotiated_property_set — the builder emits the negotiated
    set, not the wish list.
  • test_fail_closed_error_reports_why_the_probe_failed — the refusal names the systemd
    error.

Existing coverage is unchanged: test_wraps_in_systemd_scope_when_supervisor_and_available
still passes because the builder asks for the full set when no probe has run.

Local run (Windows dev box, so the POSIX-only class is skipped there — the four new
tests were executed against the same module through an identical standalone mirror,
6 passed): pytest tests/tools/test_process_registry.py tests/cron/test_restart_safe_worker.py
→ 88 passed, 6 failed, and those same 6 fail identically on unmodified main
(Windows-only baseline noise: TestTerminateHostPidPosix, PTY, stdin-EOF).
ruff check clean on both files.

Closes #102486

`_systemd_run_user_scope_available()` probes the scope backend with the
exact property set the worker spawn will use, and reads any non-zero exit
as "systemd-run --user --scope is unavailable". One of those properties,
`OOMPolicy=`, only became valid on *scope* units in systemd v253. Every
manager older than that -- Ubuntu 22.04's 249, Debian 12 and RHEL 9's 252 --
answers the probe with `Unknown assignment: OOMPolicy=kill` and exits 1
before creating anything.

On those hosts the probe therefore reports a working scope backend as
missing, caches that verdict, and `restart_safe_gateway_child_argv()` fails
closed on every dispatch. A hardening knob nobody asked about becomes a
total cron and Kanban worker outage, and the operator sees only the verdict.

Probe the desired set, and when systemd names a property it cannot parse,
drop that property and probe again. The surviving set is cached and
`_build_systemd_scope_argv()` spawns with exactly it, so the spawn can no
longer ask for something the probe never proved. Managers that do accept
`OOMPolicy=kill` keep it: an unsupported knob costs that knob, not the
isolation every dispatch depends on.

Failures that are not a property rejection -- a missing user D-Bus session
above all -- are left alone: they are real unavailability, and retrying with
a smaller set would only spin. That case still fails closed, and the refusal
now quotes the systemd error that produced it instead of only the verdict.

NousResearch#102357 identified `OOMPolicy=` as the rejected property from a live host;
this change keeps that property wherever the manager supports it instead of
removing it fleet-wide.

Closes NousResearch#102486

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@alt-glitch alt-glitch added type/bug Something isn't working P1 High — major feature broken, no workaround comp/cron Cron scheduler and job management comp/gateway Gateway runner, session dispatch, delivery comp/tools Tool registry, model_tools, toolsets backend/local Local shell execution sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages labels Sep 3, 2026
@Enough1122

Copy link
Copy Markdown

AI code review — automated review for reference; please use your judgment.

PR #102508 — Negotiate the systemd properties a transient scope accepts

Verdict: Looks good. A rejected hardening property (e.g. OOMPolicy on systemd 249/252) no longer takes down the whole restart-safe scope backend; the probe degrades gracefully and workers use the proven set.

What the change does

  • tools/process_registry.py:153 — _SCOPE_PROPERTY_NAMES (MemoryAccounting, MemoryMax, OOMPolicy) + negotiated _SYSTEMD_SCOPE_PROPERTIES / _LAST_ERROR state; _scope_property_argv builds --property pairs in order.
  • process_registry.py:252 — probe retries minus rejected properties (_rejected_scope_properties matches only unknown-assignment/property markers, case-insensitive, per-candidate name in stderr); missing-bus failures are NOT retried; each attempt uses a fresh unit name.
  • process_registry.py:325 — worker argv uses the negotiated set (full set when probe never ran = old behavior); fail-closed error now includes the systemd reason (:351).

Non-blocking

  1. process_registry.py:199 — rejection matching requires the property NAME to appear in stderr; a systemd that reports Unknown assignment without echoing the name yields () → treated as genuine unavailability (fail-closed, no retry). Safe direction. Non-blocking.
  2. process_registry.py:292 — drops ALL rejected names at once then re-probes; if two properties are rejected, one extra probe instead of two. Optimal. Non-blocking.
  3. Probe-failure TTL/caching unchanged; negotiated properties cached alongside availability. If systemd is upgraded mid-process, the cache still holds the degraded set until TTL expiry — same staleness as before, acceptable. Non-blocking.

Tests: test_process_registry.py:9 — OOMPolicy-negotiation (2 probes, surviving set, distinct unit names), no-negotiation on missing bus, worker argv uses negotiated set, fail-closed error names the cause. Covers the failure taxonomy. Good.

@kshitijk4poor

Copy link
Copy Markdown

Thanks @JoaoMarcos44 — this is the most thorough of the cluster: the Unknown assignment: X= parser, fresh --unit per attempt, refusing to negotiate away a missing user bus, and surfacing systemd's stderr in the fail-closed error are all real improvements to the probe.

Closing in favour of #102357 (@gkd2323c, earliest for #102486). The property set today is MemoryAccounting, MemoryMax, OOMPolicy; the first two are accepted by every systemd that has systemd-run --scope, and the third is rejected on scope units across the reported range (239/245/249). Once OOMPolicy=kill is simply dropped there is no remaining property to negotiate, so the generic negotiation loop would ship with zero live branches. The salvage bar here is minimal LOC, and #102357 is a two-line removal. Two pieces of yours are worth keeping independently of this cluster — including systemd's stderr in the RuntimeError, and the probe/spawn argv drift fix — and are noted as a follow-up candidate with your credit. Will reopen if #102357 does not land.

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

Labels

backend/local Local shell execution comp/cron Cron scheduler and job management comp/gateway Gateway runner, session dispatch, delivery comp/tools Tool registry, model_tools, toolsets P1 High — major feature broken, no workaround sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: restart-safe cron worker dispatch fails closed on systemd 249 — OOMPolicy=kill rejected as unknown assignment

4 participants