From 0d6a1dcb53a4338e84a5abfa22ad0313d55620f0 Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Sun, 5 Jul 2026 23:19:44 -0700 Subject: [PATCH] fix(install): trap SIGINT so Ctrl+C exits cleanly during upgrade doctor (#76386) * style: restore exec approval e2e formatting * fix(install): trap SIGINT so Ctrl+C exits cleanly during upgrade doctor Three changes to fix the install script's Ctrl+C handling: 1. Add INT/TERM signal traps that clean up temp files and exit with the correct signal exit codes (130 for SIGINT, 143 for SIGTERM). 2. Preserve signal exit codes (>128) through run_quiet_step so the doctor path can distinguish user cancellation from normal errors. Non-signal failures still return 1, preserving existing caller semantics for all other installer steps. 3. Fix guardCancel in onboard-helpers.ts: exit(0) changed to exit(1) so Clack prompt cancellation (Escape/Ctrl+C) is treated as failure, not success. This prevents the installer from continuing with plugin updates after the user explicitly cancelled. Signed-off-by: Sebastien Tardif * fix(install): abort dashboard launch on doctor cancellation When a user cancels the interactive upgrade-doctor prompt (Clack cancellation exits 1, SIGINT exits 130), clear should_open_dashboard so the installer does not launch a dead dashboard after an incomplete upgrade. Also propagate non-zero exit from run_doctor() so the non-interactive upgrade path correctly skips dashboard launch on failure. * fix: guard every run_doctor caller and add focused tests The existing-config path called run_doctor without checking its return value, so a failed or cancelled doctor would still launch the dashboard. Now both run_doctor call sites guard the return value with if-then. Adds focused tests verifying: every run_doctor caller is guarded, dashboard flag is cleared on doctor failure, signal exit codes propagate through run_quiet_step, and SIGINT (exit 130) triggers abort_install_int. * retrigger proof check * fix: exit 130 on Clack cancellation so installer treats it as SIGINT guardCancel now exits with 130 (SIGINT convention) instead of 1. When the user presses Ctrl+C at an interactive doctor prompt, the installer sees doctor_exit=130 and calls abort_install_int, aborting cleanly instead of continuing after exit 1. Signed-off-by: Sebastien Tardif * fix: narrow exit 130 to doctor-prompter path only Revert guardCancel to exit 0 by default (matching main) and pass exit code 130 only from doctor-prompter where the installer needs to distinguish user cancellation from normal failures. This preserves the existing cancellation behavior for configure, wizard, gateway, and daemon prompts while keeping the SIGINT convention for the installer's doctor subprocess. Signed-off-by: Sebastien Tardif --------- Signed-off-by: Sebastien Tardif Co-authored-by: Peter Steinberger --- scripts/install.sh | 64 ++++++++++++++++++++++++++++----- src/commands/doctor-prompter.ts | 6 ++++ src/commands/onboard-helpers.ts | 4 +-- test/scripts/install-sh.test.ts | 50 ++++++++++++++++++++++++++ 4 files changed, 113 insertions(+), 11 deletions(-) diff --git a/scripts/install.sh b/scripts/install.sh index 6a7a3bf22d99..4a83e495dbda 100755 --- a/scripts/install.sh +++ b/scripts/install.sh @@ -33,6 +33,21 @@ cleanup_tmpfiles() { } trap cleanup_tmpfiles EXIT +abort_install_int() { + cleanup_tmpfiles + echo "" + ui_warn "Installation interrupted" + exit 130 +} +abort_install_term() { + cleanup_tmpfiles + echo "" + ui_warn "Installation terminated" + exit 143 +} +trap abort_install_int INT +trap abort_install_term TERM + mktempfile() { local f f="$(mktemp)" @@ -496,12 +511,15 @@ run_quiet_step() { log="$(mktempfile)" local showed_progress=false + local cmd_exit=0 + if [[ -n "$GUM" ]] && gum_is_tty && ! is_shell_function "${1:-}"; then local cmd_quoted="" local log_quoted="" printf -v cmd_quoted '%q ' "$@" printf -v log_quoted '%q' "$log" - if run_with_spinner "$title" bash -c "${cmd_quoted}>${log_quoted} 2>&1"; then + run_with_spinner "$title" bash -c "${cmd_quoted}>${log_quoted} 2>&1" || cmd_exit=$? + if (( cmd_exit == 0 )); then return 0 fi showed_progress=true @@ -509,7 +527,8 @@ run_quiet_step() { # Keep users informed even when gum spinner cannot run (for example shell functions). ui_info "${title}" showed_progress=true - if "$@" >"$log" 2>&1; then + "$@" >"$log" 2>&1 || cmd_exit=$? + if (( cmd_exit == 0 )); then return 0 fi fi @@ -522,6 +541,12 @@ run_quiet_step() { if [[ -s "$log" ]]; then tail -n 80 "$log" >&2 || true fi + # Preserve signal exit codes (130=SIGINT, 143=SIGTERM) so callers + # like run_doctor can distinguish user cancellation from normal errors. + # Return 1 for all other failures to keep existing caller semantics. + if (( cmd_exit > 128 )); then + return "$cmd_exit" + fi return 1 } @@ -2824,7 +2849,14 @@ run_doctor() { warn_openclaw_not_found return 0 fi - run_quiet_step "Running doctor" "$claw" doctor --non-interactive || true + local doctor_exit=0 + run_quiet_step "Running doctor" "$claw" doctor --non-interactive || doctor_exit=$? + if (( doctor_exit == 130 )); then + abort_install_int + fi + if (( doctor_exit != 0 )); then + return "$doctor_exit" + fi ui_success "Doctor complete" } @@ -3195,8 +3227,9 @@ main() { run_doctor_after=true fi if [[ "$run_doctor_after" == "true" ]]; then - run_doctor - should_open_dashboard=true + if run_doctor; then + should_open_dashboard=true + fi fi # Step 7: If BOOTSTRAP.md is still present in the workspace, resume onboarding @@ -3280,10 +3313,22 @@ main() { fi ui_info "Running openclaw doctor" local doctor_ok=0 + local doctor_exit=0 if (( ${#doctor_args[@]} )); then - OPENCLAW_UPDATE_IN_PROGRESS=1 "$claw" doctor "${doctor_args[@]}" { @@ -103,6 +107,7 @@ export function createDoctorPrompter(params: { message: stylePromptMessage(confirmParams.message), }), params.runtime, + 130, ); }, select: async (p: Parameters[0], fallback: T) => { @@ -118,6 +123,7 @@ export function createDoctorPrompter(params: { ), }), params.runtime, + 130, ) as T; }, shouldRepair: repairMode.shouldRepair, diff --git a/src/commands/onboard-helpers.ts b/src/commands/onboard-helpers.ts index 06a184b0a3a5..7ac85609e604 100644 --- a/src/commands/onboard-helpers.ts +++ b/src/commands/onboard-helpers.ts @@ -48,10 +48,10 @@ export { detectBrowserOpenSupport, openUrl, resolveBrowserOpenCommand }; export { resolveAdvertisedControlUiLinks, resolveControlUiLinks, resolveLocalControlUiProbeLinks }; /** Handles Clack cancellation by exiting through the runtime. */ -export function guardCancel(value: T | symbol, runtime: RuntimeEnv): T { +export function guardCancel(value: T | symbol, runtime: RuntimeEnv, exitCode = 0): T { if (isCancel(value)) { cancel(stylePromptTitle("Setup cancelled.") ?? "Setup cancelled."); - runtime.exit(0); + runtime.exit(exitCode); throw new Error("unreachable"); } return value; diff --git a/test/scripts/install-sh.test.ts b/test/scripts/install-sh.test.ts index 131274ad5616..8d5205a1771b 100644 --- a/test/scripts/install-sh.test.ts +++ b/test/scripts/install-sh.test.ts @@ -1698,3 +1698,53 @@ describe("install.sh duplicate OpenClaw install detection", () => { expect(result.stdout).not.toContain("Multiple OpenClaw global installs detected"); }); }); + +describe("install.sh doctor cancellation and dashboard guard", () => { + const script = readFileSync(SCRIPT_PATH, "utf8"); + + it("guards every run_doctor caller against failure", () => { + // Both run_doctor call sites must guard the return value so a + // failed or cancelled doctor does not launch the dashboard. + // The upgrade path uses: if run_doctor; then should_open_dashboard=true; fi + // The existing-config path must also guard: if run_doctor; then ... + expect(script).toContain("if run_doctor; then"); + // Ensure there is no bare "run_doctor" call followed by + // "should_open_dashboard=true" without an if-guard + const bareDoctor = /^\s+run_doctor\s*$/m; + const lines = script.split("\n"); + for (let i = 0; i < lines.length; i++) { + if (bareDoctor.test(lines[i])) { + // A bare run_doctor is only acceptable inside the run_doctor + // function definition itself, not at a call site + const context = lines.slice(Math.max(0, i - 3), i + 3).join("\n"); + if (!context.includes("run_doctor()")) { + throw new Error( + `Unguarded run_doctor call at line ${i + 1}. ` + + `All run_doctor callers must check the return value.`, + ); + } + } + } + }); + + it("clears dashboard flag when doctor fails during upgrade", () => { + // The upgrade interactive doctor path must clear should_open_dashboard + // when doctor_exit is non-zero. + expect(script).toContain("should_open_dashboard=false"); + expect(script).toContain("if (( doctor_exit != 0 )); then"); + }); + + it("propagates signal exit codes through run_quiet_step", () => { + // run_quiet_step preserves signal exit codes (130=SIGINT, 143=SIGTERM) + // so run_doctor can detect user cancellation. + expect(script).toContain("if (( cmd_exit > 128 )); then"); + expect(script).toContain('return "$cmd_exit"'); + }); + + it("aborts on SIGINT (exit 130) from doctor", () => { + // Both the run_doctor function and the interactive doctor path + // must call abort_install_int on exit code 130. + expect(script).toContain("if (( doctor_exit == 130 )); then"); + expect(script).toContain("abort_install_int"); + }); +});