Make the closing plugin rescan best-effort - #11117
Open
Mario-Mohar wants to merge 1 commit into
Open
Conversation
The four plugin commands run under `set -e` and end by asking the shell to rescan. `omarchy-shell` exits 1 when its 2s IPC timeout expires, and that is exactly what happens when the reload the command itself just triggered takes longer than two seconds. So the command dies on that line. For update and remove the work is already done at that point, so they report failure for a run that succeeded. For add and clone the abort lands before the plugin is enabled, leaving it installed but disabled with nothing printed about how to enable it. `omarchy-shell -q` exists for this and is already how ten other commands make best-effort IPC calls. It also suppresses stdout, so the `>/dev/null` goes with it. The call is still made, only its failure stops being fatal.
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
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.
Fixes #11115.
The problem
All four plugin commands run under
set -euo pipefailand end by telling the shell to reload:omarchy-shell shell rescanPlugins >/dev/nullomarchy-shellwraps every call intimeout ${OMARCHY_SHELL_IPC_TIMEOUT:-2s}and exits 1 on expiry. The reload this line triggers is exactly the thing that can take longer than two seconds, so the command aborts on its own last request.>/dev/nullonly covers stdout, soomarchy-shell is not respondingstill reaches the terminal.Measured on a machine with 9 git managed plugins: a rescan with nothing to reload takes 0.05 s, one right after a real plugin update takes 1.65 s, and a second one arriving while the first is still rebuilding hits the 2 s limit. The rescan itself always completes. Only the client gives up.
What that costs depends on what follows the call:
omarchy-plugin-updateomarchy-plugin-removeomarchy-plugin-addomarchy-plugin-cloneThe exit code is not only cosmetic. #10021 and #10326 both want plugin updates to run from
omarchy update, unattended. A step that exits 1 after doing its job would report the whole system update as failed.The add and clone rows may also be the real mechanism behind #9304, which reports a plugin landing installed but disabled and attributes it to the later
enablePlugincall. With--enable --yesboth paths produce the same output, so it is worth checking which one fires there. This PR does not close that issue.The change
-qis described inomarchy-shell's own help as "Quiet best-effort mode. Suppress output and return success even when the shell, target, method, or arguments are unavailable", and it is already how ten other commands make best-effort IPC calls (omarchy-osd,omarchy-reminder,omarchy-theme-bg-set,omarchy-update-statusamong them). None of the six plugin commands used it. It also covers stdout, so the>/dev/nullgoes with it.The call is still made. Only its failure stops being fatal.
Testing
New
test/shell.d/plugin-rescan-test.sh. Itsomarchy-shellstub reproduces the real one: without-qit printsomarchy-shell is not respondingand exits 1, with-qit exits 0 silently. Six assertions covering all four commands, plus one that counts the logged rescans so best effort cannot quietly become no effort.Each of the four fixes was reverted on its own to confirm the test fails without it, and fails on the matching assertion:
./test/allbefore and after this change fails the same 6 of its test files, none of them plugin related:config,locate,runtime-smoke,screenshot-sanity,snapper,unowned-system-paths. They want anomarchy-pkgscheckout or a live compositor, neither of which this machine has. The file count goes from 236 to 237 with the new test.Also verified against the running desktop:
omarchy plugin updatewith two real plugins behind their origin exits 0 and stays quiet, where before it printed the error and exited 1.