Skip to content

Commit 0d6a1dc

Browse files
SebTardifsteipete
SebTardif
and
steipete
authored
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 <[email protected]> * 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 <[email protected]> * 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 <[email protected]> --------- Signed-off-by: Sebastien Tardif <[email protected]> Co-authored-by: Peter Steinberger <[email protected]>
1 parent 98ca8c9 commit 0d6a1dc

4 files changed

Lines changed: 113 additions & 11 deletions

File tree

‎scripts/install.sh‎

Lines changed: 55 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,21 @@ cleanup_tmpfiles() {
3333
}
3434
trap cleanup_tmpfiles EXIT
3535

36+
abort_install_int() {
37+
cleanup_tmpfiles
38+
echo ""
39+
ui_warn "Installation interrupted"
40+
exit 130
41+
}
42+
abort_install_term() {
43+
cleanup_tmpfiles
44+
echo ""
45+
ui_warn "Installation terminated"
46+
exit 143
47+
}
48+
trap abort_install_int INT
49+
trap abort_install_term TERM
50+
3651
mktempfile() {
3752
local f
3853
f="$(mktemp)"
@@ -496,20 +511,24 @@ run_quiet_step() {
496511
log="$(mktempfile)"
497512
local showed_progress=false
498513

514+
local cmd_exit=0
515+
499516
if [[ -n "$GUM" ]] && gum_is_tty && ! is_shell_function "${1:-}"; then
500517
local cmd_quoted=""
501518
local log_quoted=""
502519
printf -v cmd_quoted '%q ' "$@"
503520
printf -v log_quoted '%q' "$log"
504-
if run_with_spinner "$title" bash -c "${cmd_quoted}>${log_quoted} 2>&1"; then
521+
run_with_spinner "$title" bash -c "${cmd_quoted}>${log_quoted} 2>&1" || cmd_exit=$?
522+
if (( cmd_exit == 0 )); then
505523
return 0
506524
fi
507525
showed_progress=true
508526
else
509527
# Keep users informed even when gum spinner cannot run (for example shell functions).
510528
ui_info "${title}"
511529
showed_progress=true
512-
if "$@" >"$log" 2>&1; then
530+
"$@" >"$log" 2>&1 || cmd_exit=$?
531+
if (( cmd_exit == 0 )); then
513532
return 0
514533
fi
515534
fi
@@ -522,6 +541,12 @@ run_quiet_step() {
522541
if [[ -s "$log" ]]; then
523542
tail -n 80 "$log" >&2 || true
524543
fi
544+
# Preserve signal exit codes (130=SIGINT, 143=SIGTERM) so callers
545+
# like run_doctor can distinguish user cancellation from normal errors.
546+
# Return 1 for all other failures to keep existing caller semantics.
547+
if (( cmd_exit > 128 )); then
548+
return "$cmd_exit"
549+
fi
525550
return 1
526551
}
527552

@@ -2824,7 +2849,14 @@ run_doctor() {
28242849
warn_openclaw_not_found
28252850
return 0
28262851
fi
2827-
run_quiet_step "Running doctor" "$claw" doctor --non-interactive || true
2852+
local doctor_exit=0
2853+
run_quiet_step "Running doctor" "$claw" doctor --non-interactive || doctor_exit=$?
2854+
if (( doctor_exit == 130 )); then
2855+
abort_install_int
2856+
fi
2857+
if (( doctor_exit != 0 )); then
2858+
return "$doctor_exit"
2859+
fi
28282860
ui_success "Doctor complete"
28292861
}
28302862

@@ -3195,8 +3227,9 @@ main() {
31953227
run_doctor_after=true
31963228
fi
31973229
if [[ "$run_doctor_after" == "true" ]]; then
3198-
run_doctor
3199-
should_open_dashboard=true
3230+
if run_doctor; then
3231+
should_open_dashboard=true
3232+
fi
32003233
fi
32013234

32023235
# Step 7: If BOOTSTRAP.md is still present in the workspace, resume onboarding
@@ -3280,10 +3313,22 @@ main() {
32803313
fi
32813314
ui_info "Running openclaw doctor"
32823315
local doctor_ok=0
3316+
local doctor_exit=0
32833317
if (( ${#doctor_args[@]} )); then
3284-
OPENCLAW_UPDATE_IN_PROGRESS=1 "$claw" doctor "${doctor_args[@]}" </dev/null && doctor_ok=1
3318+
OPENCLAW_UPDATE_IN_PROGRESS=1 "$claw" doctor "${doctor_args[@]}" </dev/null || doctor_exit=$?
32853319
else
3286-
OPENCLAW_UPDATE_IN_PROGRESS=1 "$claw" doctor </dev/tty && doctor_ok=1
3320+
OPENCLAW_UPDATE_IN_PROGRESS=1 "$claw" doctor </dev/tty || doctor_exit=$?
3321+
fi
3322+
if (( doctor_exit == 130 )); then
3323+
abort_install_int
3324+
fi
3325+
# Clear dashboard flag if the doctor was cancelled or failed,
3326+
# since the upgrade did not complete successfully.
3327+
if (( doctor_exit != 0 )); then
3328+
should_open_dashboard=false
3329+
fi
3330+
if (( doctor_exit == 0 )); then
3331+
doctor_ok=1
32873332
fi
32883333
if (( doctor_ok )); then
32893334
ui_info "Updating plugins"
@@ -3307,8 +3352,9 @@ main() {
33073352
local config_path="${OPENCLAW_CONFIG_PATH:-$effective_home/.openclaw/openclaw.json}"
33083353
if [[ -f "${config_path}" || -f "$effective_home/.clawdbot/clawdbot.json" ]]; then
33093354
ui_info "Config already present; running doctor"
3310-
run_doctor
3311-
should_open_dashboard=true
3355+
if run_doctor; then
3356+
should_open_dashboard=true
3357+
fi
33123358
ui_info "Config already present; skipping onboarding"
33133359
skip_onboard=true
33143360
fi

‎src/commands/doctor-prompter.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,12 +47,15 @@ export function createDoctorPrompter(params: {
4747
if (!repairMode.canPrompt) {
4848
return p.initialValue ?? false;
4949
}
50+
// Exit 130 (SIGINT convention) so the installer can distinguish
51+
// user cancellation from normal doctor failures.
5052
return guardCancel(
5153
await confirm({
5254
...p,
5355
message: stylePromptMessage(p.message),
5456
}),
5557
params.runtime,
58+
130,
5659
);
5760
};
5861

@@ -78,6 +81,7 @@ export function createDoctorPrompter(params: {
7881
message: stylePromptMessage(p.message),
7982
}),
8083
params.runtime,
84+
130,
8185
);
8286
},
8387
confirmRuntimeRepair: async (p) => {
@@ -103,6 +107,7 @@ export function createDoctorPrompter(params: {
103107
message: stylePromptMessage(confirmParams.message),
104108
}),
105109
params.runtime,
110+
130,
106111
);
107112
},
108113
select: async <T>(p: Parameters<typeof select>[0], fallback: T) => {
@@ -118,6 +123,7 @@ export function createDoctorPrompter(params: {
118123
),
119124
}),
120125
params.runtime,
126+
130,
121127
) as T;
122128
},
123129
shouldRepair: repairMode.shouldRepair,

‎src/commands/onboard-helpers.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,10 +48,10 @@ export { detectBrowserOpenSupport, openUrl, resolveBrowserOpenCommand };
4848
export { resolveAdvertisedControlUiLinks, resolveControlUiLinks, resolveLocalControlUiProbeLinks };
4949

5050
/** Handles Clack cancellation by exiting through the runtime. */
51-
export function guardCancel<T>(value: T | symbol, runtime: RuntimeEnv): T {
51+
export function guardCancel<T>(value: T | symbol, runtime: RuntimeEnv, exitCode = 0): T {
5252
if (isCancel(value)) {
5353
cancel(stylePromptTitle("Setup cancelled.") ?? "Setup cancelled.");
54-
runtime.exit(0);
54+
runtime.exit(exitCode);
5555
throw new Error("unreachable");
5656
}
5757
return value;

‎test/scripts/install-sh.test.ts‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1698,3 +1698,53 @@ describe("install.sh duplicate OpenClaw install detection", () => {
16981698
expect(result.stdout).not.toContain("Multiple OpenClaw global installs detected");
16991699
});
17001700
});
1701+
1702+
describe("install.sh doctor cancellation and dashboard guard", () => {
1703+
const script = readFileSync(SCRIPT_PATH, "utf8");
1704+
1705+
it("guards every run_doctor caller against failure", () => {
1706+
// Both run_doctor call sites must guard the return value so a
1707+
// failed or cancelled doctor does not launch the dashboard.
1708+
// The upgrade path uses: if run_doctor; then should_open_dashboard=true; fi
1709+
// The existing-config path must also guard: if run_doctor; then ...
1710+
expect(script).toContain("if run_doctor; then");
1711+
// Ensure there is no bare "run_doctor" call followed by
1712+
// "should_open_dashboard=true" without an if-guard
1713+
const bareDoctor = /^\s+run_doctor\s*$/m;
1714+
const lines = script.split("\n");
1715+
for (let i = 0; i < lines.length; i++) {
1716+
if (bareDoctor.test(lines[i])) {
1717+
// A bare run_doctor is only acceptable inside the run_doctor
1718+
// function definition itself, not at a call site
1719+
const context = lines.slice(Math.max(0, i - 3), i + 3).join("\n");
1720+
if (!context.includes("run_doctor()")) {
1721+
throw new Error(
1722+
`Unguarded run_doctor call at line ${i + 1}. ` +
1723+
`All run_doctor callers must check the return value.`,
1724+
);
1725+
}
1726+
}
1727+
}
1728+
});
1729+
1730+
it("clears dashboard flag when doctor fails during upgrade", () => {
1731+
// The upgrade interactive doctor path must clear should_open_dashboard
1732+
// when doctor_exit is non-zero.
1733+
expect(script).toContain("should_open_dashboard=false");
1734+
expect(script).toContain("if (( doctor_exit != 0 )); then");
1735+
});
1736+
1737+
it("propagates signal exit codes through run_quiet_step", () => {
1738+
// run_quiet_step preserves signal exit codes (130=SIGINT, 143=SIGTERM)
1739+
// so run_doctor can detect user cancellation.
1740+
expect(script).toContain("if (( cmd_exit > 128 )); then");
1741+
expect(script).toContain('return "$cmd_exit"');
1742+
});
1743+
1744+
it("aborts on SIGINT (exit 130) from doctor", () => {
1745+
// Both the run_doctor function and the interactive doctor path
1746+
// must call abort_install_int on exit code 130.
1747+
expect(script).toContain("if (( doctor_exit == 130 )); then");
1748+
expect(script).toContain("abort_install_int");
1749+
});
1750+
});

0 commit comments

Comments
 (0)