diff --git a/AGENTS.md b/AGENTS.md index 196a4f1..60c9b1c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -105,6 +105,9 @@ Ruixen has hit real stale-process races more than once. The house rules: synchronous call on the render thread. - Bound search/output/cache work — no unbounded scans, unbounded output buffers, or unbounded cache growth. +- GUI lifecycle jobs must survive their launching plugin being reloaded + during deployment, before the final restart. Detach the whole worker + with independent stdio (see `lib/settings-update.sh`), not just its tail. Don't reinvent this from scratch. `ruixen.launcher/search/WorkerPool.js` (see `tests/js/WorkerPool.test.js`) and `ruixen.wallpaper/GenerationGuard.js` diff --git a/docs/INSTALLATION.md b/docs/INSTALLATION.md index 19da130..6c2b286 100644 --- a/docs/INSTALLATION.md +++ b/docs/INSTALLATION.md @@ -93,6 +93,12 @@ and `uninstall.sh` accept it, and reject any option they don't recognize ./update.sh ``` +Settings' Update button runs the same updater in an independent worker, +so replacing the Settings plugin cannot interrupt its own installation +or final shell restart. Its latest output is saved in +`~/.local/state/ruixen/settings-update.log`, including failures that happen +after Settings has reloaded. + `./update.sh --dry-run` previews it first: current vs. candidate revision, then the same install plan above for whatever is currently on disk (pulling itself is skipped, so it can't preview code not yet checked out — diff --git a/lib/settings-update.sh b/lib/settings-update.sh new file mode 100644 index 0000000..3e217d9 --- /dev/null +++ b/lib/settings-update.sh @@ -0,0 +1,31 @@ +#!/usr/bin/env bash +# Keep the ENTIRE GUI update alive when deployment reloads Settings, not +# just install.sh's final restart. Only this waiting relay belongs to QML. +set -Eeuo pipefail + +repo_dir="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")/.." && pwd)" +state_dir="$HOME/.local/state/ruixen" +mkdir -p "$state_dir" + +# A reloaded Settings instance can click Update again while the first +# worker is still alive. Preserve its log; the normal lifecycle lock +# inside update.sh still protects against CLI install/update/uninstall. +exec 9>"$state_dir/settings-update.lock" +if ! flock -n 9; then + echo 'A Settings update is already running; wait for it to finish.' >&2 + exit 1 +fi + +log_file="$state_dir/settings-update.log" +# New session AND independent stdio: losing the QML Process must neither +# kill the worker's process group nor give it SIGPIPE on its next print. +# shellcheck disable=SC2016 # $1 expands in the independent child shell. +setsid bash -c 'cd -- "$1" && exec ./update.sh --acknowledge-interrupted' \ + -- "$repo_dir" "$log_file" 2>&1 & +worker_pid=$! +status=0 +wait "$worker_pid" || status=$? +if (( status != 0 )); then + tail -n 20 "$log_file" >&2 +fi +exit "$status" diff --git a/ruixen.launcher/extensions/settings/services/PluginService.qml b/ruixen.launcher/extensions/settings/services/PluginService.qml index 89cfdd5..3549cc7 100644 --- a/ruixen.launcher/extensions/settings/services/PluginService.qml +++ b/ruixen.launcher/extensions/settings/services/PluginService.qml @@ -160,12 +160,10 @@ Item { stderr: StdioCollector { waitForEnd: true onStreamFinished: { - // A successful run never gets here to report success -- - // update.sh ends with `omarchy restart shell`, which tears down - // and reloads this very plugin instance before this handler - // would ever fire. Only a failure that happens BEFORE that - // point (network down, git pull conflict, etc.) leaves this - // instance alive long enough to actually show the error. + // The relay reports failures while this instance is alive. + // Deployment itself can reload Settings BEFORE the final shell + // restart; the independent worker survives either teardown and + // keeps its output in ~/.local/state/ruixen/settings-update.log. if (updateProc.exitCode !== 0) { var raw = String(text || "").trim() var interrupted = raw.indexOf("--acknowledge-interrupted") !== -1 @@ -197,7 +195,7 @@ Item { // GUI updates should be one-click recoverable. The flag is a no-op // when no stale lifecycle journal exists; when one does, the lock // still guarantees there is no real concurrent lifecycle operation. - updateProc.command = ["bash", "-c", "cd '" + safePath + "' && ./update.sh --acknowledge-interrupted"] + updateProc.command = ["bash", "-c", "exec bash '" + safePath + "/lib/settings-update.sh'"] updateProc.running = true } @@ -252,7 +250,8 @@ Item { // Omarchy's own default if there was none), and restarts the shell. // // Deliberately fired via Quickshell.execDetached, not a lifecycle- - // bound Process like updateProc above -- the script's own last real + // bound Process -- updateProc above uses an independent worker too. + // The script's own last real // step disables/removes ruixen.launcher itself (this very plugin), // which would tear down this QML instance (and, plausibly, any // Process objects it owns) mid-script if that happened before the diff --git a/tests/run-all.sh b/tests/run-all.sh index 0d502c4..6fd5e0a 100755 --- a/tests/run-all.sh +++ b/tests/run-all.sh @@ -40,6 +40,7 @@ suites=( "$script_dir/lifecycle-journal-recovery.sh" "$script_dir/lifecycle-flag-parsing.sh" "$script_dir/settings-update-interruption.sh" + "$script_dir/settings-update-worker.sh" "$script_dir/install-restart-detached-tail.sh" "$script_dir/install-theme-overlays.sh" "$script_dir/lifecycle-lock.sh" diff --git a/tests/settings-update-interruption.sh b/tests/settings-update-interruption.sh index 5d509d2..4f7abdc 100755 --- a/tests/settings-update-interruption.sh +++ b/tests/settings-update-interruption.sh @@ -36,8 +36,10 @@ check "PluginService uses a distinct interrupted update status" \ "$(grep -c 'pluginUpdateStatus = interrupted ? "interrupted" : "error"' "$service_qml")" "1" check "PluginService keeps more generic error context than the old three-line truncation" \ "$(grep -c 'errLines.length - 5' "$service_qml")" "1" -check "PluginService GUI update always passes --acknowledge-interrupted" \ - "$(grep -c './update.sh --acknowledge-interrupted' "$service_qml")" "1" +check "PluginService GUI update uses the independent worker relay" \ + "$(grep -c '/lib/settings-update.sh' "$service_qml")" "1" +check "GUI worker always passes --acknowledge-interrupted" \ + "$(grep -c './update.sh --acknowledge-interrupted' "$repo_dir/lib/settings-update.sh")" "1" check "PluginService clears stale check status when update starts" \ "$(awk '/function updateRuixenShell/ { in_fn = 1 } in_fn && /function checkForUpdates/ { in_fn = 0 } in_fn && /pluginCheckStatus = ""|pluginCheckError = ""/ { count++ } END { print count + 0 }' "$service_qml")" "2" check "PluginService clears stale update status when check starts" \ diff --git a/tests/settings-update-worker.sh b/tests/settings-update-worker.sh new file mode 100755 index 0000000..79bc173 --- /dev/null +++ b/tests/settings-update-worker.sh @@ -0,0 +1,75 @@ +#!/usr/bin/env bash +# Exercise the GUI relay with a real detached worker, then destroy its +# launching process group BEFORE the fake install reaches its restart. +set -Eeuo pipefail +repo_dir="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")/.." && pwd)" +tmp="$(mktemp -d)" +trap 'rm -rf "$tmp"' EXIT +checkout="$tmp/repo with a ' quote" +mkdir -p "$checkout/lib" "$tmp/home" +cp "$repo_dir/lib/settings-update.sh" "$checkout/lib/" +export HOME="$tmp/home" +cat >"$checkout/update.sh" <<'WORKER' +#!/usr/bin/env bash +set -Eeuo pipefail +[[ "$1" == --acknowledge-interrupted ]] +echo 'worker output before Settings reload' +touch "$HOME/started" +for ((i=0; i<200; i++)); do + if [[ -e "$HOME/release" ]]; then + echo 'restart and cleanup completed after Settings reload' + touch "$HOME/completed" + exit 0 + fi + sleep 0.05 +done +exit 1 +WORKER +chmod +x "$checkout/update.sh" + +wait_for() { + local i + for ((i=0; i<200; i++)); do + [[ -e "$1" ]] && return 0 + sleep 0.05 + done + echo "FAIL - timed out waiting for $1" >&2 + return 1 +} + +setsid bash "$checkout/lib/settings-update.sh" >"$tmp/relay.out" 2>&1 & +relay_pid=$! +wait_for "$HOME/started" +# A replacement Settings instance must not overwrite the active log. +if bash "$checkout/lib/settings-update.sh" >"$tmp/second.out" 2>&1; then + echo 'FAIL - concurrent GUI update was accepted' >&2 + exit 1 +fi +grep -q 'already running' "$tmp/second.out" +grep -q 'worker output before' "$HOME/.local/state/ruixen/settings-update.log" +echo 'ok - concurrent GUI update refused without truncating the worker log' + +kill -TERM -- "-$relay_pid" +wait "$relay_pid" 2>/dev/null || true +touch "$HOME/release" +wait_for "$HOME/completed" +grep -q 'restart and cleanup completed' "$HOME/.local/state/ruixen/settings-update.log" +echo 'ok - worker survives losing its launching process group and output listener' + +# A failure before reload must still reach the existing QML error handler. +cat >"$checkout/update.sh" <<'WORKER' +#!/usr/bin/env bash +echo 'simulated git pull failure' >&2 +exit 23 +WORKER +# The marker precedes worker exit by a few instructions; wait for its lock. +for ((i=0; i<200; i++)); do + if flock -n "$HOME/.local/state/ruixen/settings-update.lock" true; then break; fi + sleep 0.05 +done +status=0 +bash "$checkout/lib/settings-update.sh" >"$tmp/failure.out" 2>&1 || status=$? +[[ "$status" == 23 ]] +grep -q 'simulated git pull failure' "$tmp/failure.out" +grep -q 'simulated git pull failure' "$HOME/.local/state/ruixen/settings-update.log" +echo 'ok - worker exit status and failure output reach the GUI relay and persistent log'