Make loading and installing the scripting addition more user-friendly

[dominiklohmann]

Feature request

The below things should all happen in order, unless the the flag --without-sa is passed to yabai.

1. yabai checks if the current version of the scripting addition is installed by comparing signatures/hashes
   • not installed: yabai requests admin access and installs the scripting addition, then loads the scripting addition
   • old version installed: yabai requests admin access, uninstalls and installs the scripting addition, then loads the scripting addition
2. yabai checks if the scripting addition is loaded (Edit: already happening)
   • not loaded: yabai loads the scripting addition automatically
3. yabai asks the scripting addition whether its pattern matching was successful
   • failed to match patterns: yabai quits and tells the user about it (or to run yabai --without-sa instead)

The goal here is to automatically detect failures, and also to automatically reinstall the scripting addition if needed (and only if needed).

Running a privileged job can be done using SMJobBless, although I have not yet investigated this very much. Apparently Apple Script can also be used, which seems way simpler for this use case. See → this StackOverflow thread for more on either method.

Thoughts?

[koekeishiya]

I have mixed feelings regarding this.

I think comparing hashes upon startup and notifying the user if the scripting-addition is not up to date is a good change that should probably be implemented.

Loading the scripting-addition during startup is already done in the current implementation.

I'm not particularly well versed in apple events, and so I am unsure if there is a way for the sender to get notified whether the bundle was loaded properly or not. If you check the code in osax/loader.m there are different possible results such as bundle is already loaded, which could cause us trouble if the already loaded bundle is an old one. To fix that and load the new bundle we must first kill the Dock process.

Reporting back to yabai when we fail to match patterns would require us to have a solution to the previous step - knowing that the bundle was loaded properly and that we should wait for some kind of status report.

As far as I know SMJobBless requires installation of a Helper which is a whole separate program that must first be installed, which also require root permissions to do. Then at any point your program can signal the helper to perform a privileged operation on your behalf without you having to request root privileges. The AppleScript sample would seem like the better option to me, provided that it actually works.

That said, I'm not a huge fan of automatically installing the scripting-addition with a user provided password. I can see this being helpful during installation of updates or something, but as a first install I personally think the users of yabai should thoroughly read through this and make an informed decision, instead of just disabling SIP and running some command because the install guide told them to.

[dominiklohmann]

To avoid the necessity of killing Dock because we cannot know whether an outdated scripting addition is still loaded, shouldn't it be possible to "version" the scripting addition by renaming it? E.g. instead of yabai.osax use yabai-<some unique id/hash>.osax. Uninstalling would then have to remove all versions of yabai-*.osax. The socket file name would need to include that same unique id/hash as well.

Also maybe a command that checks whether an up-to-date scripting addition is installed is sufficient. This is just so it's not necessary to use sudo on every update of yabai.

Edit:

I think the communication between the scripting addition and yabai can be improved greatly by using XPC over a socket file, which supports waiting for replies and bidirectional messages. This could then be used to make the scripting addition signal that it has found the required addresses.

[koekeishiya]

To avoid the necessity of killing Dock because we cannot know whether an outdated scripting addition is still loaded, shouldn't it be possible to "version" the scripting addition by renaming it? E.g. instead of yabai.osax use yabai-<some unique id/hash>.osax.

I'm not sure at this moment. I need to find time to look into some documentation on Apple Events and experiment to see what can be done.

I think the communication between the scripting addition and yabai can be improved greatly by using XPC over a socket file, which supports waiting for replies and bidirectional messages. This could then be used to make the scripting addition signal that it has found the required addresses.

This is fairly trivial to solve using a socket file as well, but we still need to know whether the actual bundle got loaded properly or not. We can sort of do this by checking if it is possible to connect through the socket file - if connection is possible we know that the bundle is loaded.

I think the following is really everything that is needed:

1. yabai is launched.
2. check if scripting-addition is installed. If
   a) yes, yabai initiates a message to retrieve the version of the loaded scripting-addition, as well as a status report based on the result of the pattern matching. If the version is outdated, or full functionality is not supported we report this to the user somehow (notification?, modal dialog?)
   b) no, show notification / modal dialog informing the user, but allow yabai to run regardless

Edit:

Step 2 would obviously be ran again if the Dock is restarted and we try to load the scripting-addition that way.

Edit2:

If someone is aware of a method that we can use to detect the status of SIP without having to parse the text output from csrutil status that would be great. Last time I looked into this it required writing a kernel extension to be able to call said function.

[dominiklohmann]

This is turning into a discussion about the implementation robustness of the scripting addition, and I like it.

we still need to know whether the actual bundle got loaded properly or not.

Assuming bidirectional communication: If the scripting addition replies after yabai asks for the status report based on the result of the pattern matching, it should be fully loaded. Or am I missing something here?

On the same note: This comes with the added benefit of allowing yabai to get return values of all the do_* functions in payload.m.

2. (notification?, modal dialog?)

System notification definitely. Could even use the logo here. If I recall correctly Apple advises against using modal dialogs in the human interface guidelines for macOS.

If someone is aware of a method that we can use to detect the status of SIP without having to parse the text output from csrutil status that would be great. Last time I looked into this it required writing a kernel extension to be able to call said function.

You just need to call it once, I don't think this is neeeded. Just run [[ "$(csrutil status)" == *"enabled." ]] through /bin/bash, check whether the exit-code is zero and be done with it. By doing the parsing in bash directly you won't have to deal with it in C.

[koekeishiya]

I have implemented the groundwork for detecting various form of failures, as well as validating the scripting-addition version and located patterns. Currently they just log some crappy messages to stdout/stderr.

[koekeishiya]

To avoid the necessity of killing Dock because we cannot know whether an outdated scripting addition is still loaded, shouldn't it be possible to "version" the scripting addition by renaming it? E.g. instead of yabai.osax use yabai-<some unique id/hash>.osax. Uninstalling would then have to remove all versions of yabai-*.osax. The socket file name would need to include that same unique id/hash as well.

From what I've been able to figure out, loading the bundle using NSBundle has the same issue as when loading a shared object file manually with dlopen. When it uses the obj-c stdlib it will not be able to reload the code as long as the object file has an identical name.

A workaround for this is to store the payload bundle with a different name, similar to what I did in https://github.com/koekeishiya/chunkwm/pull/206 but the symlink hack won't work here.
This would also require us to be able to actually unload the existing code properly, without causing the Dock to crash which I think is more challenging than it may seem at first. I've previously been unable to do thread_join(..) to perform cleanup for example without the Dock simply triggering a restart.

because we cannot know whether an outdated scripting addition is still loaded

After the changes mentioned in my previous comment we can now query the loaded scripting-addition's version and whether or not it has located all the different functions and internal instances that we require, so in some way we can actually determine if the loaded instance is working correctly or not, and just send a kill -9 to the Dock if necessary.

[dominiklohmann]

Looks good. Minor note:

send a kill -9 to the Dock if necessary.

I don't think sending SIGKILL is a good idea, because Dock writes to multiple property list files, which may then get messed up. A normal kill with SIGTERM should be enough.

Before killing the Dock yabai needs to store what windows were minimised and minimise them again once Dock is back up and running.

[koekeishiya]

Before killing the Dock yabai needs to store what windows were minimised and minimise them again once Dock is back up and running.

I'm not sure if this can be done reliably as we don't have this information available at the time we try to load the scripting-addition. We could defer loading the scripting-addition, but to know about all created windows the user would have to visit every existing space.

If the user manually executes yabai --load-sa we also don't have this information as it launches a new process to perform that operation (so that an already running instance is not required simply to trigger a load). The --load-sa command is somewhat redundant as there are very few cases where it is actually useful to have.

[dominiklohmann]

Alright, so just noting in the docs+wiki that loading the scripting addition will sometimes deminimise windows it is then.

[koekeishiya]

Added user notifications and shortened messages to make them more readable in a notification format.

[koekeishiya]

I believe most of what we discussed has been implemented now. I still don't think we should request root privileges to automatically install/uninstall the scripting-addition. I'll consider this resolved, unless something comes up during testing.

[dominiklohmann]

Just one more question on my end:

If I run sudo yabai --install-sa and there's an older version already installed, will it override an existing installation of the scripting addition and kill Dock if the older version was also loaded?

---

Tip: The notification settings need to be adjusted for Dock.app to affect the scripting addition.

[koekeishiya]

If I run sudo yabai --install-sa and there's an older version already installed, will it override an existing installation of the scripting addition and kill Dock if the older version was also loaded?

It should overwrite an existing installation, but I have noticed that it does not always manage to, for some reason - or I simply manage to mess up my routine sometimes?? It also won't kill the Dock.

However, if you do something like:

# do whatever to get an updated version of yabai

sudo yabai --uninstall-sa
sudo yabai --install-sa

# do whatever to launch yabai, this will trigger a Dock restart, causing the new scripting-addition to be loaded.

Tip: The notification settings need to be adjusted for Dock.app to affect the scripting addition.

Huh didn't even know this was a thing, it just worked for me. I don't have to use the Dock's bundle identifier. I could just write like com.koekeishia.yabai and the notification still works. It does not seem to load the icon of the executable though - so it doesn't look as good. I have added and tested code that pulls in a different main image, but the other image is still shown next to the title, although it is a lot smaller than it normally would be.

[dominiklohmann]

It does not seem to load the icon of the executable though - so it doesn't look as good. I have added and tested code that pulls in a different main image, but the other image is still shown next to the title, although it is a lot smaller than it normally would be.

That's the content image. Overriding the app image can only be done from a private API. Look at the source code of terminal-notifier for this.

[koekeishiya]

Looks like this:

Pretty sure I did override the app image.

[dominiklohmann]

Afaik the content image is optional and if set, it's the blue icon in the left. If no content image is set, the app image is used as the content image instead of the tiny image left of the title.

[koekeishiya]

I took a look at terminal-notifier. They set the image the same way:

  if(options[@"appIcon"]){
    [userNotification setValue:[self getImageFromURL:options[@"appIcon"]] forKey:@"_identityImage"];
    [userNotification setValue:@(false) forKey:@"_identityImageHasBorder"];
  }

Also is there a reason why I still get notifications when using the Dock bundle id even if I have disabled the option that you showed in the Notifications preference pane for the Dock? Do I need to reboot or something for these changes to take effect?

[dominiklohmann]

At the very top of the Notifications preference pane for Dock, set the style to None. The setting I highlighted only makes it so the notifications don't show up in notification center.

[koekeishiya]

I see. Well, either we use the dock bundle identifier, or we use the custom one and have to live with the looks shown in the screenshot above. I happen to have Alerter installed as I used this tool previously, and it has the same problem when specifying -appIcon.

[koekeishiya]

I decided it is more correct to specify our own bundle identifier. To "solve" the issue with the mini icon that remains visible when we add our own to the left-side, simply toggle the option "Badge app icon" in the Notifications entry for yabai.

That is likely the last change I do regarding this issue, unless someone brings new ideas that bring value.

[dominiklohmann]

That is likely the last change I do regarding this issue, unless someone brings new ideas that bring value.

Actually have one for my auto-update script. yabai --check-sa, which returns a non-zero exit code if the payload needs to be reinstalled, so I don't need to prompt for privileges every time.

[koekeishiya]

I'll probably hash the file to verify that, as the scripting-addition is not necessarily loaded at the time you want to validate if it is the latest version or not, or do you think the existing method is good enough for this purpose?

[dominiklohmann]

Existing method should be good enough I think. It's just that I don't want to prompt for password/touch id every time. You can safely assume that yabai is already running.

[koekeishiya]

I'll see what works best. It occurred to me that I can simply just read the version of out the bundles plist file as well. I think that is the correct solution in this case, where we don't care about the located patterns, but simply want to control versioning.

[koekeishiya]

If I run sudo yabai --install-sa and there's an older version already installed, will it override an existing installation of the scripting addition and kill Dock if the older version was also loaded?

FYI: On the latest master sudo yabai --install-sa seems to properly override an old installation for me (as expected by the code). I've also made it so that the Dock is restarted when the scripting-addition is installed, #135 .