From 572bd355f8787b50f0d7407c0525f041baef66c1 Mon Sep 17 00:00:00 2001 From: 7418 <7418@openclaw.ai> Date: Fri, 13 Feb 2026 13:51:15 +0800 Subject: [PATCH] fix: require explicit user confirmation before install, cancel on close, tree-kill on Windows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit P0: Add "confirm" phase — wizard checks prereqs then shows Install button, user must explicitly click before any brew/winget/npm command runs. P1: Closing the wizard dialog while install is running now auto-calls install:cancel to stop the backend process. P2: On Windows, use taskkill /T /F /PID to kill the entire process tree instead of just the shell wrapper, ensuring npm/winget actually stops. Co-Authored-By: Claude Opus 4.6 --- electron/main.ts | 22 +++- src/components/layout/InstallWizard.tsx | 146 +++++++++++++++++------- 2 files changed, 126 insertions(+), 42 deletions(-) diff --git a/electron/main.ts b/electron/main.ts index 9fa5ca5..4936757 100644 --- a/electron/main.ts +++ b/electron/main.ts @@ -669,14 +669,23 @@ app.whenReady().then(async () => { } installState.status = 'cancelled'; + installState.logs.push('Cancelling installation...'); if (installProcess) { + const pid = installProcess.pid; try { - installProcess.kill(); + if (process.platform === 'win32' && pid) { + // Windows: kill entire process tree (shell: true spawns cmd.exe which + // spawns npm/winget — child.kill() only kills the shell, not the tree) + spawn('taskkill', ['/T', '/F', '/PID', String(pid)], { stdio: 'ignore' }); + } else { + installProcess.kill(); + } } catch { // already dead } installProcess = null; + installState.logs.push('Installation process terminated.'); } mainWindow?.webContents.send('install:progress', installState); @@ -738,9 +747,16 @@ app.on('activate', async () => { }); app.on('before-quit', async (e) => { - // Kill any running install process + // Kill any running install process (tree-kill on Windows) if (installProcess) { - try { installProcess.kill(); } catch { /* already dead */ } + const pid = installProcess.pid; + try { + if (process.platform === 'win32' && pid) { + spawn('taskkill', ['/T', '/F', '/PID', String(pid)], { stdio: 'ignore' }); + } else { + installProcess.kill(); + } + } catch { /* already dead */ } installProcess = null; } diff --git a/src/components/layout/InstallWizard.tsx b/src/components/layout/InstallWizard.tsx index 42fca37..903cb1e 100644 --- a/src/components/layout/InstallWizard.tsx +++ b/src/components/layout/InstallWizard.tsx @@ -18,6 +18,7 @@ import { LoaderIcon, CircleIcon, CopyIcon, + DownloadIcon, } from "lucide-react"; interface InstallProgress { @@ -40,22 +41,25 @@ interface InstallWizardProps { type WizardPhase = | "checking" + | "confirm" | "already-installed" | "installing" | "success" | "failed"; +interface PrereqResult { + hasNode: boolean; + nodeVersion?: string; + hasClaude: boolean; + claudeVersion?: string; +} + function getInstallAPI() { if (typeof window !== "undefined") { // eslint-disable-next-line @typescript-eslint/no-explicit-any return (window as any).electronAPI?.install as | { - checkPrerequisites: () => Promise<{ - hasNode: boolean; - nodeVersion?: string; - hasClaude: boolean; - claudeVersion?: string; - }>; + checkPrerequisites: () => Promise; start: (options?: { includeNode?: boolean }) => Promise; cancel: () => Promise; getLogs: () => Promise; @@ -92,6 +96,7 @@ export function InstallWizard({ const [progress, setProgress] = useState(null); const [logs, setLogs] = useState([]); const [copied, setCopied] = useState(false); + const [prereqs, setPrereqs] = useState(null); const logEndRef = useRef(null); const cleanupRef = useRef<(() => void) | null>(null); @@ -103,6 +108,17 @@ export function InstallWizard({ scrollToBottom(); }, [logs, scrollToBottom]); + // Cancel backend install and clean up listener + const cancelInstall = useCallback(async () => { + const api = getInstallAPI(); + if (!api) return; + try { + await api.cancel(); + } catch { + // ignore cancel errors + } + }, []); + const startInstall = useCallback(async (options?: { includeNode?: boolean }) => { const api = getInstallAPI(); if (!api) return; @@ -138,9 +154,11 @@ export function InstallWizard({ setPhase("checking"); setLogs(["Checking environment..."]); setProgress(null); + setPrereqs(null); try { const result = await api.checkPrerequisites(); + setPrereqs(result); if (result.hasClaude) { setLogs((prev) => [ @@ -152,37 +170,34 @@ export function InstallWizard({ return; } - if (!result.hasNode) { - setLogs((prev) => [ - ...prev, - "Node.js not found. Will attempt to install Node.js and Claude Code...", - ]); - startInstall({ includeNode: true }); - } else { + // Don't auto-install — show confirmation first + if (result.hasNode) { setLogs((prev) => [ ...prev, `Node.js ${result.nodeVersion} found.`, - "Claude Code not found. Starting installation...", + "Claude Code CLI not detected.", + ]); + } else { + setLogs((prev) => [ + ...prev, + "Node.js not found.", + "Claude Code CLI not detected.", ]); - startInstall(); } + setPhase("confirm"); } catch (err: unknown) { setPhase("failed"); const msg = err instanceof Error ? err.message : String(err); setLogs((prev) => [...prev, `Error checking prerequisites: ${msg}`]); } - }, [startInstall]); - - const handleCancel = useCallback(async () => { - const api = getInstallAPI(); - if (!api) return; - try { - await api.cancel(); - } catch { - // ignore cancel errors - } }, []); + // User explicitly clicks "Install" — only then start the actual install + const handleConfirmInstall = useCallback(() => { + const needsNode = prereqs ? !prereqs.hasNode : false; + startInstall({ includeNode: needsNode }); + }, [prereqs, startInstall]); + const handleCopyLogs = useCallback(async () => { try { await navigator.clipboard.writeText(logs.join("\n")); @@ -198,6 +213,17 @@ export function InstallWizard({ onInstallComplete?.(); }, [onOpenChange, onInstallComplete]); + // [P1] Close dialog = cancel running install + const handleOpenChange = useCallback( + async (nextOpen: boolean) => { + if (!nextOpen && phase === "installing") { + await cancelInstall(); + } + onOpenChange(nextOpen); + }, + [phase, cancelInstall, onOpenChange] + ); + // Auto-check when dialog opens useEffect(() => { if (open) { @@ -205,6 +231,7 @@ export function InstallWizard({ setLogs([]); // eslint-disable-line react-hooks/set-state-in-effect setProgress(null); // eslint-disable-line react-hooks/set-state-in-effect setCopied(false); // eslint-disable-line react-hooks/set-state-in-effect + setPrereqs(null); // eslint-disable-line react-hooks/set-state-in-effect checkPrereqs(); } return () => { @@ -218,17 +245,19 @@ export function InstallWizard({ const steps = progress?.steps ?? []; return ( - + Install Claude Code - Automatically install Claude Code CLI + {phase === "confirm" + ? "Claude Code CLI was not detected. Install it now?" + : "Automatically install Claude Code CLI"}
- {/* Step list */} + {/* Step list (only during/after install) */} {steps.length > 0 && (
{steps.map((step) => ( @@ -258,7 +287,7 @@ export function InstallWizard({
)} - {/* Phase-specific messages */} + {/* Phase: checking */} {phase === "checking" && steps.length === 0 && (
@@ -266,6 +295,32 @@ export function InstallWizard({
)} + {/* Phase: confirm — ask user before installing */} + {phase === "confirm" && ( +
+
+ {prereqs && !prereqs.hasNode && ( +

+ Node.js — not found (will be installed via {process.platform === "win32" ? "winget" : "Homebrew"}) +

+ )} + {prereqs?.hasNode && ( +

+ Node.js {prereqs.nodeVersion} — found +

+ )} +

+ Claude Code CLI — not found +

+
+

+ Click Install to automatically set up{" "} + {prereqs && !prereqs.hasNode ? "Node.js and " : ""}Claude Code CLI. +

+
+ )} + + {/* Phase: already-installed */} {phase === "already-installed" && (
@@ -280,6 +335,7 @@ export function InstallWizard({
)} + {/* Phase: success */} {phase === "success" && (
@@ -310,28 +366,40 @@ export function InstallWizard({
- + {logs.length > 0 && ( + + )} + {/* Confirm phase: single "Install" button */} + {phase === "confirm" && ( + + )} + + {/* Installing: cancel button */} {phase === "installing" && ( - )} + {/* Failed: retry */} {phase === "failed" && ( )} + {/* Success / already-installed: done */} {(phase === "success" || phase === "already-installed") && (