fix(activate): exit non-zero when units weren't applied, 4 when they failed by jeremy · Pull Request #582 · numtide/system-manager

jeremy · 2026-10-12T23:35:56Z

system-manager-engine activate (and a profile's bin/activate) exits 0 when systemd didn't apply the profile's units, so a caller can't tell an applied profile from a half-applied one. We hit this running system-manager unattended: an upgrade loop switches, checks health and rolls back on a regression. On exit 0 it records the new generation as applied while the old unit is still running, or the new one has failed.

What fails today

All of these log the error, write the state file and exit 0:

A job systemd refuses. For example, a changed unit with a bad setting: systemd refuses to restart it, and the old version keeps running. This is the TODO in for_each_unit.
A userborn.service that ran and failed. A oneshot's job completes either way, so restart_userborn_if_exists returned Ok, and users the profile declares may not exist.
A services phase that returns WithPartialResult, from a job timeout or a D-Bus error.

A managed unit that fails to start after an otherwise complete activation also exits 0.

Minimal repro:

systemd.services.broken = {
  wantedBy = [ "system-manager.target" ];
  serviceConfig.Type = "oneshot";
  script = "exit 1";
};

sudo ./result/bin/activate; echo $?   # 0 on main, with broken.service failed

What changes

The control flow is unchanged. Every unit is still attempted, and the state file still records what was applied, so activating another profile (a rollback) starts from the truth. Only the result changes:

Exit 1 (ActivationFailure::Incomplete) when part of the profile wasn't applied. The error names each part: the refused jobs (for_each_unit now collects them, for sysinit-reactivation.target too), a failed userborn, a partial services phase.
Exit 4 (ActivationFailure::UnitsFailed) when everything was applied but managed (non-masked) units are in the failed state afterwards. The error names the units.
main finds the failure through anyhow's downcast_ref. Every other error still exits 1, as before.

One case had to be told apart from a refusal. When a profile drops a unit that has already exited, such as a oneshot without RemainAfterExit, its file is gone by the time it's stopped, and systemd answers the stop with NoSuchUnit ("not loaded"). There's nothing to stop. On main that was logged as an error; now it's logged at debug and doesn't count.

Exit codes

Code	Meaning
0	The profile was applied and no managed unit is failed.
1	Part of the profile wasn't applied (or any other error, as today).
4	The profile was applied, and these managed units are failed afterwards.

4 is the code NixOS' switch-to-configuration uses for the same case. It lets a caller judge the failed units against what was already failing before the switch, instead of treating them like a switch that didn't happen. One difference: switch-to-configuration reports every failed unit on the system; this reports only the profile's own.

Compatibility

Anything that ran bin/activate or the engine and relied on exit 0 after a refused job or a failed unit will now see 1 or 4. That's the point, but it's a visible change, hence the CHANGELOG entry.
The system-manager CLI is unchanged: invoke_engine turns any non-zero engine status into its own exit 1 ("Engine command exited with status 4"). So system-manager switch exits 1 in both cases, and the 4 reaches only callers of bin/activate or the engine. Passing the engine's code through seems right to me, but I left it out to keep this small.
system-manager-upgrade.service (system.autoUpgrade) runs system-manager switch, so an upgrade that leaves a unit failed now fails the upgrade unit too.

Tests

Unit tests: activate::tests::failed_units_exit_like_nixos_switch, the_exit_code_survives_anyhow (the code survives the trip through anyhow to main), and services::tests::a_unit_systemd_has_not_loaded_needs_no_stop.
A new container test, activation-exit-codes:
- a complete activation exits 0;
- a profile with a failing oneshot exits 4 and names it;
- going back exits 0, dropping a oneshot that had already exited;
- a profile that changes a running unit to one with a bad setting (a simple service with two ExecStart= lines) exits 1 and names the unit, while the profile's other units keep running;
- going back exits 0.

cargo test --workspace, cargo clippy --all-targets (no warnings), cargo fmt --check and nixfmt --check pass locally. register::tests::test_try_nix_eval needs nix on PATH, and passes with it. I couldn't run the container test here, since my Nix can't provide uid-range. It evaluates, its profiles build, and its script passes the derivation's mypy and pyflakes steps. I checked the systemd behaviour it relies on by hand, on systemd 261: a changed unit with two ExecStart= lines loads as bad-setting, and ReloadOrRestartUnit is refused while the old process keeps running; stopping an exited unit whose file is gone gets NoSuchUnit. CI will be the judge of the rest.

/etc

This PR leaves /etc alone. An /etc entry that can't be written, and an /etc phase that fails outright, still exit 0 on this branch. #551 fixes both, and I'd rather not compete with it. The two touch different code. Merged together they conflict in one place, where both add a mod tests to the end of activate.rs. #551's error exits 1 through main unchanged.

Things you may want differently

The 30 s timeout in wait_for_jobs. A start job that runs longer, such as a slow oneshot, already ended the services phase early; now that also exits 1. A configurable timeout, or telling "still running" apart from "failed", may suit you better.
A unit that was already failed. Exit 4 counts every managed unit that is failed after the switch, including one that failed before it and wasn't restarted. NixOS does the same, and it's why 4 is separate from 1, but a caller has to compare against the state before the switch.
Exit code 4 itself. If you'd rather have a single non-zero code with the units in the message, the split is one match in ActivationFailure::exit_code.
prepopulate still exits 0 when registering services returns a partial result. It starts nothing, so I left it out; it's two lines if you want it.

Fixes #581

chatgpt-codex-connector · 2026-10-12T23:36:06Z

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-15T18:41:27.604636Z	`1d42883`	New commits

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

…failed An activation in which systemd refused a job, userborn.service failed or the services phase ended early (a job timeout, a D-Bus error) logged the error, wrote the state and exited 0. A caller judging the switch, a person or an unattended upgrade, then took a half-applied profile for an applied one. The control flow is unchanged: every unit is still attempted and the state file still records what was applied. Only the result changes: - exit 1 (ActivationFailure::Incomplete) when part of the profile wasn't applied, naming each part: a job systemd refused (the TODO in for_each_unit), including for sysinit-reactivation.target; a failed userborn.service (a oneshot's job finishes whether or not it succeeded, so its result is checked); a services phase that returned a partial result; - exit 4 (ActivationFailure::UnitsFailed) when everything was applied and managed units are in the failed state afterwards, naming them, as NixOS' switch-to-configuration does, so a caller can tell the two apart and judge the second against what was failing before. Stopping a unit systemd no longer has loaded (one that had exited, whose file the new profile dropped) fails with NoSuchUnit. That isn't a refusal, there is nothing to stop, so it is now logged at debug instead of as an error. /etc is left as it is: numtide#551 covers it.

Copilot

🟡 Changes recommended

Wait-error paths discard concurrently collected refused-unit diagnostics.

1 open finding

Include refused units in stop wait error diagnostics · New

What changed in this PR

Ensures activation returns meaningful non-zero statuses when systemd only partially applies a profile or managed units fail.

Changes:

Adds activation failure types and exit codes 1 and 4.
Detects refused jobs, failed userborn, and failed managed units.
Adds regression tests and release notes.

File	Description
`crates/system-manager-engine/src/activate.rs`	Aggregates activation failures and checks failed units.
`crates/system-manager-engine/src/activate/services.rs`	Detects refused jobs and failed systemd units.
`crates/system-manager-engine/src/main.rs`	Maps activation failures to exit statuses.
`testFlake/container-tests/activation-exit-codes.nix`	Tests activation exit-code behavior.
`CHANGELOG.md`	Documents the visible behavior change.

🧠 Review effort: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7b95a75333

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

A job systemd accepted was taken as done once it was removed, whatever its result. Removing a managed mount that can't be unmounted, for instance: the stop job fails, the mount stays, and as the profile no longer lists it, the failed-units check doesn't look at it either, so the activation exited 0. The job monitor now records each job's result from JobRemoved, and matches jobs by object path rather than unit name, so that another job on the same unit (one ours replaced, reported as canceled) isn't read as ours. `done` completed a job, and so did `skipped` (the job didn't apply to the unit's state: nothing to do). Any other result (`failed`, `dependency`, `timeout`, `canceled`, ...) means the unit isn't where the job was to put it. Which exit that gives follows from what it means for the profile: - a stop of a unit the profile drops or masks: exit 1, "<unit> not stopped (job <result>)". The old unit may still run, and nothing else reports it; - a restart of sysinit-reactivation.target or userborn.service: exit 1, as their refusals already are. For userborn this replaces the failed-state check: a oneshot's job fails with it, and the result also covers a dependency that failed or a job that timed out; - a (re)start or reload of a unit the profile keeps: exit 4, alongside the failed units, "<unit> (job <result>)". The unit was applied and didn't come up; that is what 4 already means, and NixOS' switch-to-configuration also exits 4 for a timeout, failed or dependency job result. Exit 1 here would double count: a unit whose start fails is also in the failed state, and a pre-existing failure would turn every later activation that touches it into an incomplete one. A unit both failed and with a failed job is named once. This also reports what the failed-state check can't see: a reload that fails leaves the unit active on its old configuration, and a start a dependency failed leaves it inactive. When a wait is cut short (a job timeout, a D-Bus error), the error returned now names what wasn't applied before it, refusals and stops that didn't complete, rather than only the timeout. The container test adds a reload that fails (exit 4) and a dropped mount kept busy (exit 1).

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4da6670502

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

jeremy · 2026-10-14T06:11:10Z

CI here has been red since I opened this, and I didn't notice until now. Sorry for the noise.

Every nixbot failure I've read has the same cause. The container tests' activation exits 4 because suid-sgid-wrappers.service fails, even under empty-config. On main that unit most likely fails the same way, and the exit 0 hides it. This PR is what makes a failed managed unit exit 4.

I'm finding out whether the unit fails on real Ubuntu and Debian hosts or only inside the test containers, and I'll report back here. If it's only the containers, I'd suggest splitting this PR:

units that weren't applied exit 1, on their own;
failed units exit 4, in a follow-up once the containers are sorted.

I'm happy to go whichever way you prefer.

When a unit the profile drops didn't stop (its stop job failed, or systemd refused it), the activation exited 1, but the state it wrote was the new profile's services, without that unit. Activating the same profile again then had nothing to stop and nothing to check: the unit still ran, and the activation exited 0. Deactivating didn't stop it either. The state now keeps the old entry of each unit the profile drops whose stop didn't complete, so that: - activating the profile again retries the stop, and exits 1 again until it completes; - deactivating stops the unit; - a rollback to a profile that has the unit finds the same entry, and leaves the unit as it is rather than restarting it. When the wait is cut short (a job timeout, a D-Bus error), which stops completed isn't known, so every unit the profile drops keeps its entry: retrying a stop that already completed does nothing. A unit the profile masks keeps its new, masked entry; masked units are stopped on every activation anyway. The container test now activates the profile again after the failed stop (exit 1 again), rolls back to the profile with the mount (exit 0, the mount untouched), and, once the mount is free, activates again, which unmounts it.

…tainers The container tests run systemd-nspawn inside the Nix build sandbox, and the sandbox's seccomp filter (Nix's filter-syscalls, on by default) refuses setuid and setgid bits, and file capabilities, to every process under it. The container's root is no exception, with every capability: chmod: changing permissions of '/run/wrappers/wrappers.LIVKfoT1vC/mount': Operation not permitted suid-sgid-wrappers.service: Failed with result 'exit-code'. so suid-sgid-wrappers.service has failed on every activation in every container test, empty-config included. Nothing looked: activation exits 0 whatever the units do, and no container test checks the wrappers. Outside the sandbox the unit works: the VM tests' security-wrappers check asserts it succeeds, on every image. The driver now puts a drop-in in each container's /etc that skips the unit in a container, so that a container test's activation reflects the profile rather than the sandbox. It's in the driver, not in each test's configuration, because several tests build their own profiles, and so do projects that use the driver to test theirs. It's written when the container is prepared rather than baked into the rootfs, which would rebuild every distribution's image for one file.

… needs The VM tests' newConfig, and the system-path container test's, declare a sops secret decrypted with /run/age-keys.txt. The example VM test copies the key there first; the prepopulate VM test and the system-path container test didn't, so activating newConfig there failed sops-install-secrets.service, and with it the restart of sysinit-reactivation.target: sops-install-secrets: cannot read keyfile '/run/age-keys.txt': open /run/age-keys.txt: no such file or directory The activation's exit status doesn't report a job that didn't complete, and neither test looked for the secret, so both passed. They now copy the key as the example test does, and check that the secret is there.

Every activation restarts userborn.service twice: through sysinit-reactivation.target, which requires it, and then on its own before tmpfiles. systemd's default start limit is 5 starts in 10 seconds, so the third activation within ten seconds or so finds it reached, and systemd refuses the restart: userborn.service: Start request repeated too quickly. userborn.service: Failed with result 'start-limit-hit'. userborn doesn't run, so the users and groups of that profile aren't applied, and userborn.service is left failed. The VM tests' example and prepopulate scenarios activate that fast, and the activation exited 0 all the same: a restart job's result isn't read. Activation now resets the failed state of each unit the profile manages before restarting any of them, which also resets its start limit, as NixOS' switch-to-configuration does (for every unit there). Only the profile's units: on a host that isn't NixOS, the other units' failed states are the host's, and someone may be watching them.

jeremy · 2026-10-14T10:57:52Z

Following up on the red CI. #582 doesn't cause any of the three failures behind it. All three exist on main today, and activation's exit 0 hides them. A diagnostic run on main, which dumps each unit's journal after every activation, shows all three with exit 0.

suid-sgid-wrappers.service fails in every container test. This is the test environment, not the wrappers module. The containers run inside the Nix build sandbox, whose seccomp filter refuses setuid and setgid bits even to the container's root:
```
chmod: changing permissions of '/run/wrappers/wrappers.…/mount': Operation not permitted
```
The VM tests (vm-*-security-wrappers) pass with the unit succeeding. Fix: 328a43f has the container test driver skip the unit inside containers (ConditionVirtualization=!container). It's written when each container is prepared, so the distro images don't rebuild.
userborn.service hits systemd's start limit. This one is a real bug. Every activation restarts userborn twice, once through sysinit-reactivation.target and once on its own. So a third activation within about 10 s gets start-limit-hit (5 starts in 10 s), and that profile's users aren't applied. vm-*-example does exactly that. Fix: 91c4728 resets the failed state, and with it the start limit, of the profile's own units before restarting them. NixOS resets every unit; I kept it to the profile's units so a host's own failures stay visible. Happy to do it differently if you'd prefer NixOS' behaviour, or one userborn restart per activation.
A missing test key. vm-*-prepopulate and container-*-system-path activate a config with a sops secret but never copy the age key, so sysinit-reactivation.target fails its dependency. Fix: 66203e9 copies the key and checks that the secret exists.

With those three and the state fix for the open thread (04f7e65), every x86_64 check is green on my fork's hosted runners. That covers all VM tests and the containers on Debian 13 and Ubuntu 24.04 and 26.04. The exception is Ubuntu 22.04: its containers never finish booting on GitHub's hosted runners, on any branch, main with a no-op change included. nixbot booted them fine before, so I'll rely on its run for 22.04.

So I'm not splitting this after all. Splitting wouldn't help: failures 2 and 3 come through the exit-1 path, not exit 4.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 91c472893c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-10-14T11:10:10Z

+        return e
+            .downcast_ref::<system_manager_engine::activate::ActivationFailure>()
+            .map_or(ExitCode::FAILURE, |failure| {
+                ExitCode::from(failure.exit_code())


Propagate exit code 4 through the public CLI

When activation is invoked through the normal system-manager switch/activate path (locally or over SSH), this new exit code is consumed by crates/system-manager/src/main.rs:731-746, which turns every nonzero engine status into a generic anyhow error, and that binary's handler at lines 913-918 always returns exit 1. Consequently only the low-level engine/profile script exposes 4, while callers of the public CLI still cannot distinguish failed units from an incomplete activation; preserve the child status through the wrapper.

Useful? React with 👍 / 👎.

Not doing this here. It's true that system-manager switch turns any non-zero engine status into its own exit 1, but that is deliberate in this PR (see Compatibility in the description): passing the code through changes the CLI's local and SSH paths, and what system-manager-upgrade.service sees, and is easier to review on its own. Happy to open it as a follow-up if the maintainers want it.

…restart Two ways an activation still exited 0 with part of the profile not applied. A new unit is started only through system-manager.target, and StartUnit returns only the target's own job. When a unit the profile doesn't manage, and that a managed unit requires, fails, the managed unit's start job ends with "dependency": the unit is inactive, not failed, and the target's job completes all the same. Activation now starts the target with EnqueueUnitJob, which also returns the jobs queued in the same transaction, and waits for those of managed units. One that doesn't complete is reported with the failed units (exit 4), as a restart that doesn't complete already is. A changed unit that systemd refuses to restart, or whose restart or reload doesn't complete, still runs its old configuration, but the state recorded its new entry. Activating the same profile again found nothing changed, didn't retry, and exited 0. Such a unit now keeps its old entry, as a dropped unit whose stop doesn't complete does, so that the next activation retries it. When the wait is cut short, which restarts completed isn't known, and every changed unit keeps its old entry. The activation-exit-codes container test covers both: activating the bad-setting profile a second time exits 1 again, and a unit whose unmanaged dependency fails exits 4 and is named.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 847ff7d936

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

… applied The state recorded a unit's new entry unless the activation noticed that its stop or restart didn't complete, and each early return had to know which units to hold back. The stop phase's didn't for changed units: when the stop wait timed out, a changed unit was recorded on its new entry before its restart was even queued, and activating the profile again found nothing to restart. The rule is now the other way round. A unit the profile drops or changes is recorded on its old entry until systemd confirms its stop or restart, and only then on the profile's. Whatever cuts the activation short, the state holds what was applied, and the next activation retries the rest. That also covers a restart or reload that ran and didn't complete without making the activation incomplete (exit 4): the unit keeps its old entry and is retried, which the container test now checks.

jeremy · 2026-10-15T21:27:02Z

nixbot's build of 1d42883 (build 79) failed every VM test, 20 of 20, with RuntimeError: Shell did not start in time. The guests stall at "Loading initial ramdisk", before boot finishes, so system-manager never runs in any of them. The same tests passed on 847ff7d (build 78) and on main at 10:29 UTC today. Could you rerun build 79 when nixbot's quieter?

Copilot AI balanced review requested due to automatic review settings October 12, 2026 23:35

jeremy force-pushed the jeremy/581 branch from 7b95a75 to a634ec8 Compare October 12, 2026 23:36

jeremy mentioned this pull request Oct 12, 2026

fix(activate): fail when an /etc file could not be written #551

Open

Copilot started reviewing on behalf of jeremy October 12, 2026 23:36

View session

Conversation

jeremy commented Oct 12, 2026

What fails today

What changes

Exit codes

Compatibility

Tests

/etc

Things you may want differently

Uh oh!

chatgpt-codex-connector Bot commented Oct 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codex Review Summary

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

jeremy commented Oct 14, 2026

Uh oh!

jeremy commented Oct 14, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

chatgpt-codex-connector Bot Oct 14, 2026

Choose a reason for hiding this comment

Uh oh!

jeremy Oct 15, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

jeremy commented Oct 15, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

chatgpt-codex-connector Bot commented Oct 12, 2026 •

edited

Loading