Skip to content

Provide More Intuitive Behaviour for Launchd Services #40

Merged
chipsenkbeil merged 3 commits intochipsenkbeil:mainfrom
jacderida:fix-launchd_on_success
Dec 15, 2025
Merged

Provide More Intuitive Behaviour for Launchd Services #40
chipsenkbeil merged 3 commits intochipsenkbeil:mainfrom
jacderida:fix-launchd_on_success

Conversation

@jacderida
Copy link
Collaborator

@jacderida jacderida commented Dec 14, 2025

  • b6fda30 fix: use correct logic for launchd restart policies

    For the OnSuccess case, the logic was actually inverted, so the service was restarting when the
    exit was not successful. This has now been corrected.

    The logic we were previously using for the OnSuccess case has now been applied to the OnFailure
    case, so we now support that scenario rather than emitting a warning saying we don't support it.

    A comment clarifies that when the SuccessfulExit construct is not used, this is effectively the
    Always policy.

  • da5d943 feat: prevent launchd services starting when keepalive is used

    When the service definition includes the KeepAlive setting it causes services to automatically
    start when launchctl load is used. This is quite unintuitive and is not how services behave on
    other platforms.

    We now make use of the Disabled setting to prevent the service starting automatically and make it
    behave like service managers on other platforms. This is desirable when you want to add many
    services first and not have them all start at the same time, which is the case for our application.
    It also seems to be the behaviour most of our users expect and I personally think makes most sense.

  • 652d31a chore(release): bump to version 0.10.0

For the `OnSuccess` case, the logic was actually inverted, so the service was restarting when the
exit was *not* successful. This has now been corrected.

The logic we were previously using for the `OnSuccess` case has now been applied to the `OnFailure`
case, so we now support that scenario rather than emitting a warning saying we don't support it.

A comment clarifies that when the `SuccessfulExit` construct is *not* used, this is effectively the
`Always` policy.
@jacderida
Copy link
Collaborator Author

Hi Chip,

I realise this is another breaking change quite soon, but I really think it's worth it.

Due to the addition of the KeepAlive key, our macOS users encountered strange behaviour because Launchd was starting services as soon as they were added. This change makes the service manager behave like service managers on other platforms, i.e., the service will not start when it has been installed, and needs to be started explicitly as a separate step. I think the consistent cross-platform behaviour is desirable and the way it's done on other platforms just makes more sense anyway.

A little bug was also fixed.

It looks like I have some breaking tests here. Will try to investigate.

When the service definition includes the `KeepAlive` setting it causes services to automatically
start when `launchctl load` is used. This is quite unintuitive and is not how services behave on
other platforms.

We now make use of the `Disabled` setting to prevent the service starting automatically and make it
behave like service managers on other platforms. This is desirable when you want to add many
services first and not have them all start at the same time, which is the case for our application.
It also seems to be the behaviour most of our users expect and I personally think makes most sense.
@jacderida jacderida force-pushed the fix-launchd_on_success branch from 4f353e1 to 5344091 Compare December 14, 2025 21:39
@jacderida
Copy link
Collaborator Author

I've now fixed the failing test case.

The Launchd test for running as sudo but with a specific user is still broken. Unfortunately I still don't have time to look into this at the moment, but do intend to when I get some free time.

@jacderida jacderida force-pushed the fix-launchd_on_success branch from 5344091 to 652d31a Compare December 14, 2025 21:44
@chipsenkbeil chipsenkbeil merged commit 840a78d into chipsenkbeil:main Dec 15, 2025
11 of 13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants