mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-12 21:53:00 -06:00
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 <sebtardif@ncf.ca> * 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 <sebtardif@ncf.ca> * 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 <sebtardif@ncf.ca> --------- Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca> Co-authored-by: Peter Steinberger <steipete@gmail.com>
This commit is contained in:
+55
-9
@@ -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[@]}" </dev/null && doctor_ok=1
|
||||
OPENCLAW_UPDATE_IN_PROGRESS=1 "$claw" doctor "${doctor_args[@]}" </dev/null || doctor_exit=$?
|
||||
else
|
||||
OPENCLAW_UPDATE_IN_PROGRESS=1 "$claw" doctor </dev/tty && doctor_ok=1
|
||||
OPENCLAW_UPDATE_IN_PROGRESS=1 "$claw" doctor </dev/tty || doctor_exit=$?
|
||||
fi
|
||||
if (( doctor_exit == 130 )); then
|
||||
abort_install_int
|
||||
fi
|
||||
# Clear dashboard flag if the doctor was cancelled or failed,
|
||||
# since the upgrade did not complete successfully.
|
||||
if (( doctor_exit != 0 )); then
|
||||
should_open_dashboard=false
|
||||
fi
|
||||
if (( doctor_exit == 0 )); then
|
||||
doctor_ok=1
|
||||
fi
|
||||
if (( doctor_ok )); then
|
||||
ui_info "Updating plugins"
|
||||
@@ -3307,8 +3352,9 @@ main() {
|
||||
local config_path="${OPENCLAW_CONFIG_PATH:-$effective_home/.openclaw/openclaw.json}"
|
||||
if [[ -f "${config_path}" || -f "$effective_home/.clawdbot/clawdbot.json" ]]; then
|
||||
ui_info "Config already present; running doctor"
|
||||
run_doctor
|
||||
should_open_dashboard=true
|
||||
if run_doctor; then
|
||||
should_open_dashboard=true
|
||||
fi
|
||||
ui_info "Config already present; skipping onboarding"
|
||||
skip_onboard=true
|
||||
fi
|
||||
|
||||
@@ -47,12 +47,15 @@ export function createDoctorPrompter(params: {
|
||||
if (!repairMode.canPrompt) {
|
||||
return p.initialValue ?? false;
|
||||
}
|
||||
// Exit 130 (SIGINT convention) so the installer can distinguish
|
||||
// user cancellation from normal doctor failures.
|
||||
return guardCancel(
|
||||
await confirm({
|
||||
...p,
|
||||
message: stylePromptMessage(p.message),
|
||||
}),
|
||||
params.runtime,
|
||||
130,
|
||||
);
|
||||
};
|
||||
|
||||
@@ -78,6 +81,7 @@ export function createDoctorPrompter(params: {
|
||||
message: stylePromptMessage(p.message),
|
||||
}),
|
||||
params.runtime,
|
||||
130,
|
||||
);
|
||||
},
|
||||
confirmRuntimeRepair: async (p) => {
|
||||
@@ -103,6 +107,7 @@ export function createDoctorPrompter(params: {
|
||||
message: stylePromptMessage(confirmParams.message),
|
||||
}),
|
||||
params.runtime,
|
||||
130,
|
||||
);
|
||||
},
|
||||
select: async <T>(p: Parameters<typeof select>[0], fallback: T) => {
|
||||
@@ -118,6 +123,7 @@ export function createDoctorPrompter(params: {
|
||||
),
|
||||
}),
|
||||
params.runtime,
|
||||
130,
|
||||
) as T;
|
||||
},
|
||||
shouldRepair: repairMode.shouldRepair,
|
||||
|
||||
@@ -48,10 +48,10 @@ export { detectBrowserOpenSupport, openUrl, resolveBrowserOpenCommand };
|
||||
export { resolveAdvertisedControlUiLinks, resolveControlUiLinks, resolveLocalControlUiProbeLinks };
|
||||
|
||||
/** Handles Clack cancellation by exiting through the runtime. */
|
||||
export function guardCancel<T>(value: T | symbol, runtime: RuntimeEnv): T {
|
||||
export function guardCancel<T>(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;
|
||||
|
||||
@@ -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");
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user