doctor: a service is "installed" only if the OS will actually start it - #96
Merged
Merged
Conversation
myobie
force-pushed
the
fix/doctor-checks-service-enablement-not-presence
branch
2 times, most recently
from
August 30, 2026 11:53
708779b to
4eefbf4
Compare
Finding 10 of the 2026-08-29 review. doctor set service_installed from the unit file's mere presence, so a service disabled during an incident with its unit left in place reported "installed and managed by the OS" — and after a reboot no daemon started. The CA trust check was fixed for exactly this presence-is-not-the-property mistake; the service check was not. service::service_enablement() now queries the manager: on Linux `systemctl --user is-enabled fabric.service`, on macOS whether launchd has the label loaded. It returns Enabled, PresentNotEnabled, NotInstalled, or Unknown, and doctor reports the middle state as a problem with its own repair, because "present but not enabled" and "never installed" are different faults. The output interpreters are pure and tested on any host; the subprocess call is the untestable part, like the version probe beside it. Proof: a_present_but_not_enabled_service_is_a_problem_not_ok pins that a disabled-but-present service reads as a problem and not as enabled, with the healthy case still Ok. Two interpreter tests pin the manager-output mapping.
myobie
force-pushed
the
fix/doctor-checks-service-enablement-not-presence
branch
from
August 30, 2026 12:01
4eefbf4 to
d53b5dc
Compare
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
1 participant
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.
Finding 10 of the 2026-08-29 review.
The hole
gathersetservice_installed = Some(service_unit_path().exists()). Presence of the unit file is not enablement. A service disabled during an incident (systemctl --user disable, orlaunchctl disable) with its unit file left in place reportedok service installed and managed by the OS. After a reboot, no daemon starts, and the last thing anyone read said it was fine.src/ca.rswas fixed for exactly this presence-is-not-the-property mistake (is_installedverifies rather than checks a path); the service check was not.The fix
service::service_enablement()queries the manager and returnsEnabled | PresentNotEnabled | NotInstalled | Unknown:systemctl --user is-enabled fabric.service.launchctl print <target>reports the label loaded.Doctor reports
PresentNotEnabledas a problem ("the service is installed but not enabled, so it will not start after a reboot") distinct fromNotInstalled, because the two have different repairs.Tests
The output interpreters (
interpret_systemd_is_enabled,interpret_launchd) are pure andcfg-free, so they run on any host: they pin thatenabledreads as enabled whiledisabled/static/masked/linked/empty read as present-not-enabled, and that a missing unit isNotInstalledwhatever the query says.a_present_but_not_enabled_service_is_a_problem_not_okpins the diagnose-level verdict. The subprocess call itself is the untestable part, like the version probe beside it.Full suite green locally on macOS.