diff --git a/cli/src/agent/backends/acp/AcpStdioTransport.ts b/cli/src/agent/backends/acp/AcpStdioTransport.ts index e74f7681..b2178b95 100644 --- a/cli/src/agent/backends/acp/AcpStdioTransport.ts +++ b/cli/src/agent/backends/acp/AcpStdioTransport.ts @@ -1,5 +1,6 @@ import { spawn, type ChildProcessWithoutNullStreams } from 'node:child_process'; import { logger } from '@/ui/logger'; +import { killProcessByChildProcess } from '@/utils/process'; interface JsonRpcRequest { jsonrpc: '2.0'; @@ -107,7 +108,7 @@ export class AcpStdioTransport { async close(): Promise { this.process.stdin.end(); - this.process.kill(); + await killProcessByChildProcess(this.process); this.rejectAllPending(new Error('ACP transport closed')); } @@ -140,7 +141,7 @@ export class AcpStdioTransport { logger.debug('[ACP] Failed to parse JSON-RPC line', { line, error }); this.rejectAllPending(protocolError); this.process.stdin.end(); - this.process.kill(); + void killProcessByChildProcess(this.process); return; } diff --git a/cli/src/claude/sdk/query.ts b/cli/src/claude/sdk/query.ts index 816a1bed..585f1199 100644 --- a/cli/src/claude/sdk/query.ts +++ b/cli/src/claude/sdk/query.ts @@ -24,6 +24,7 @@ import { } from './types' import { getDefaultClaudeCodePath, getCleanEnv, logDebug, streamToStdin } from './utils' import { withBunRuntimeEnv } from '@/utils/bunRuntime' +import { killProcessByChildProcess } from '@/utils/process' import type { Writable } from 'node:stream' import { logger } from '@/ui/logger' @@ -367,7 +368,7 @@ export function query(config: { // Setup cleanup const cleanup = () => { if (!child.killed) { - child.kill('SIGTERM') + void killProcessByChildProcess(child) } } diff --git a/cli/src/codex/codexLocal.ts b/cli/src/codex/codexLocal.ts index 23ab1728..baa33595 100644 --- a/cli/src/codex/codexLocal.ts +++ b/cli/src/codex/codexLocal.ts @@ -1,6 +1,7 @@ import { spawn } from 'node:child_process'; import { logger } from '@/ui/logger'; import { restoreTerminalState } from '@/ui/terminalState'; +import { killProcessByChildProcess } from '@/utils/process'; /** * Filter out 'resume' subcommand which is managed internally by hapi. @@ -76,7 +77,7 @@ export async function codexLocal(opts: { if (child.exitCode === null && !child.killed) { logger.debug('[CodexLocal] Abort timeout reached, sending SIGKILL'); try { - child.kill('SIGKILL'); + void killProcessByChildProcess(child, true); } catch (error) { logger.debug('[CodexLocal] Failed to send SIGKILL:', error); } diff --git a/cli/src/codex/codexMcpClient.ts b/cli/src/codex/codexMcpClient.ts index 8dee0843..522e66f2 100644 --- a/cli/src/codex/codexMcpClient.ts +++ b/cli/src/codex/codexMcpClient.ts @@ -5,6 +5,7 @@ import { Client } from '@modelcontextprotocol/sdk/client/index.js'; import { StdioClientTransport } from '@modelcontextprotocol/sdk/client/stdio.js'; import { logger } from '@/ui/logger'; +import { isProcessAlive, killProcess } from '@/utils/process'; import type { CodexSessionConfig, CodexToolResponse } from './types'; import { z } from 'zod'; import { ElicitRequestSchema } from '@modelcontextprotocol/sdk/types.js'; @@ -435,11 +436,10 @@ export class CodexMcpClient { // As a last resort, if child still exists, send SIGKILL if (pid) { - try { - process.kill(pid, 0); // check if alive + if (isProcessAlive(pid)) { logger.debug('[CodexMCP] Child still alive, sending SIGKILL'); - try { process.kill(pid, 'SIGKILL'); } catch {} - } catch { /* not running */ } + await killProcess(pid, true); + } } this.transport = null; diff --git a/cli/src/daemon/README.md b/cli/src/daemon/README.md index f8b46eff..dd810fba 100644 --- a/cli/src/daemon/README.md +++ b/cli/src/daemon/README.md @@ -55,7 +55,7 @@ The daemon detects when CLI binary changes (e.g., after `npm upgrade hapi`): Every 60 seconds (configurable via `HAPI_DAEMON_HEARTBEAT_INTERVAL`): 1. **Guard**: Skips if previous heartbeat still running (prevents concurrent heartbeats) -2. **Session Pruning**: Checks each tracked PID with `process.kill(pid, 0)`, removes dead sessions +2. **Session Pruning**: Checks each tracked PID with `isProcessAlive(pid)`, removes dead sessions 3. **Version Check**: Compares CLI binary mtime, triggers self-restart if changed 4. **PID Ownership**: Verifies daemon still owns state file, self-terminates if another daemon took over 5. **State Update**: Writes `lastHeartbeat` timestamp to daemon.state.json @@ -74,7 +74,7 @@ Control Flow: - Stops HTTP server - Deletes daemon.state.json - Releases lock file -5. If HTTP fails, falls back to `process.kill(pid, 'SIGKILL')` +5. If HTTP fails, falls back to `killProcess(pid, true)` (uses `taskkill /T /F` on Windows) ## 2. Multi-Agent Support @@ -135,7 +135,7 @@ When spawning a session, directory handling: Via RPC `stop-session` or HTTP `/stop-session`: 1. `stopSession()` finds session by `happySessionId` or `PID-{pid}` format -2. Sends SIGTERM to process (via `childProcess.kill()` or `process.kill(pid)`) +2. Sends termination request via `killProcessByChildProcess()` or `killProcess()` (Windows uses `taskkill /T`) 3. `on('exit')` handler removes from tracking map ## 4. HTTP Control Server (Fastify) diff --git a/cli/src/daemon/controlClient.ts b/cli/src/daemon/controlClient.ts index d88325b3..f312687e 100644 --- a/cli/src/daemon/controlClient.ts +++ b/cli/src/daemon/controlClient.ts @@ -10,6 +10,7 @@ import packageJson from '../../package.json'; import { existsSync, statSync } from 'node:fs'; import { join } from 'node:path'; import { isBunCompiled, projectPath } from '@/projectPath'; +import { isProcessAlive, killProcess } from '@/utils/process'; export function getInstalledCliMtimeMs(): number | undefined { if (isBunCompiled()) { @@ -42,9 +43,7 @@ async function daemonPost(path: string, body?: any): Promise<{ error?: string } }; } - try { - process.kill(state.pid, 0); - } catch (error) { + if (!isProcessAlive(state.pid)) { const errorMessage = 'Daemon is not running, file is stale'; logger.debug(`[CONTROL CLIENT] ${errorMessage}`); return { @@ -143,14 +142,13 @@ export async function checkIfDaemonRunningAndCleanupStaleState(): Promise { const start = Date.now(); while (Date.now() - start < timeout) { - try { - process.kill(pid, 0); + if (isProcessAlive(pid)) { await new Promise(resolve => setTimeout(resolve, 100)); - } catch { - return; // Process is dead + continue; } + return; // Process is dead } throw new Error('Process did not die within timeout'); } diff --git a/cli/src/daemon/daemon.integration.test.ts b/cli/src/daemon/daemon.integration.test.ts index b889518e..f2d397a4 100644 --- a/cli/src/daemon/daemon.integration.test.ts +++ b/cli/src/daemon/daemon.integration.test.ts @@ -32,6 +32,7 @@ import { readDaemonState, clearDaemonState } from '@/persistence'; import { Metadata } from '@/api/types'; import { spawnHappyCLI } from '@/utils/spawnHappyCLI'; import { getLatestDaemonLog } from '@/ui/logger'; +import { isProcessAlive, isWindows, killProcess, killProcessByChildProcess } from '@/utils/process'; // Utility to wait for condition async function waitFor( @@ -234,7 +235,7 @@ describe.skipIf(!await isServerHealthy())('Daemon Integration Tests', { timeout: // Also kill the terminal process directly to be sure try { - terminalHappyProcess.kill('SIGTERM'); + await killProcessByChildProcess(terminalHappyProcess); } catch (e) { // Process might already be dead } @@ -325,18 +326,13 @@ describe.skipIf(!await isServerHealthy())('Daemon Integration Tests', { timeout: const initialLogs = readdirSync(logsDir).filter(f => f.endsWith('-daemon.log')); // Send SIGKILL to daemon (force kill) - process.kill(daemonPid, 'SIGKILL'); + await killProcess(daemonPid, true); // Wait for process to die await new Promise(resolve => setTimeout(resolve, 500)); // Check if process is dead - let isDead = false; - try { - process.kill(daemonPid, 0); - } catch { - isDead = true; - } + const isDead = !isProcessAlive(daemonPid); expect(isDead).toBe(true); // Check that log file exists (it was created when daemon started) @@ -350,36 +346,38 @@ describe.skipIf(!await isServerHealthy())('Daemon Integration Tests', { timeout: await clearDaemonState(); }); - it('should die with cleanup logs when SIGTERM is sent', async () => { - // SIGTERM test - daemon should cleanup gracefully + it('should die with cleanup logs when a graceful shutdown is requested', async () => { + // Graceful shutdown test - daemon should cleanup gracefully const logFile = await getLatestDaemonLog(); if (!logFile) { throw new Error('No log file found'); } - // Send SIGTERM to daemon (graceful shutdown) - process.kill(daemonPid, 'SIGTERM'); + if (isWindows()) { + // Windows taskkill does not deliver SIGTERM/SIGBREAK to Node handlers. + await stopDaemonHttp(); + } else { + // Send SIGTERM to daemon (graceful shutdown) + await killProcess(daemonPid); + } // Wait for graceful shutdown await new Promise(resolve => setTimeout(resolve, 4_000)); // Check if process is dead - let isDead = false; - try { - process.kill(daemonPid, 0); - } catch { - isDead = true; - } + const isDead = !isProcessAlive(daemonPid); expect(isDead).toBe(true); // Read the log file to check for cleanup messages const logContent = readFileSync(logFile.path, 'utf8'); // Should contain cleanup messages - expect(logContent).toContain('SIGTERM'); + if (!isWindows()) { + expect(logContent).toContain('SIGTERM'); + } expect(logContent).toContain('cleanup'); - console.log('[TEST] Daemon terminated gracefully with SIGTERM - cleanup logs written'); + console.log('[TEST] Daemon terminated gracefully - cleanup logs written'); // Clean up state file if it still exists (should have been cleaned by SIGTERM handler) await clearDaemonState(); diff --git a/cli/src/daemon/doctor.ts b/cli/src/daemon/doctor.ts index 7b12f734..ea052000 100644 --- a/cli/src/daemon/doctor.ts +++ b/cli/src/daemon/doctor.ts @@ -6,7 +6,7 @@ */ import psList from 'ps-list'; -import spawn from 'cross-spawn'; +import { killProcess } from '@/utils/process'; /** * Find all HAPI CLI processes (including current process) @@ -92,25 +92,17 @@ export async function killRunawayHappyProcesses(): Promise<{ killed: number, err try { console.log(`Killing runaway process PID ${pid}: ${command}`); - if (process.platform === 'win32') { - // Windows: use taskkill - const result = spawn.sync('taskkill', ['/F', '/PID', pid.toString()], { stdio: 'pipe' }); - if (result.error) throw result.error; - if (result.status !== 0) throw new Error(`taskkill exited with code ${result.status}`); - } else { - // Unix: try SIGTERM first - process.kill(pid, 'SIGTERM'); - - // Wait a moment - await new Promise(resolve => setTimeout(resolve, 1000)); - - // Check if still alive - const processes = await psList(); - const stillAlive = processes.find(p => p.pid === pid); - if (stillAlive) { - console.log(`Process PID ${pid} ignored SIGTERM, using SIGKILL`); - process.kill(pid, 'SIGKILL'); - } + await killProcess(pid, false); + + // Wait a moment + await new Promise(resolve => setTimeout(resolve, 1000)); + + // Check if still alive + const processes = await psList(); + const stillAlive = processes.find(p => p.pid === pid); + if (stillAlive) { + console.log(`Process PID ${pid} ignored termination request, using force kill`); + await killProcess(pid, true); } console.log(`Successfully killed runaway process PID ${pid}`); diff --git a/cli/src/daemon/install.ts b/cli/src/daemon/install.ts index ffd2af3d..b15092fd 100644 --- a/cli/src/daemon/install.ts +++ b/cli/src/daemon/install.ts @@ -2,6 +2,10 @@ import { logger } from '@/ui/logger'; import { install as installMac } from './mac/install'; export async function install(): Promise { + if (process.platform === 'win32') { + throw new Error('Daemon installation as Windows service not yet supported. Use "hapi daemon start".'); + } + if (process.platform !== 'darwin') { throw new Error('Daemon installation is currently only supported on macOS'); } diff --git a/cli/src/daemon/run.ts b/cli/src/daemon/run.ts index 3b4e8570..500370d5 100644 --- a/cli/src/daemon/run.ts +++ b/cli/src/daemon/run.ts @@ -12,6 +12,7 @@ import packageJson from '../../package.json'; import { getEnvironmentInfo } from '@/ui/doctor'; import { spawnHappyCLI } from '@/utils/spawnHappyCLI'; import { writeDaemonState, DaemonLocallyPersistedState, readDaemonState, acquireDaemonLock, releaseDaemonLock } from '@/persistence'; +import { isProcessAlive, isWindows, killProcess, killProcessByChildProcess } from '@/utils/process'; import { cleanupDaemonState, getInstalledCliMtimeMs, isDaemonRunningCurrentlyInstalledHappyVersion, stopDaemon } from './controlClient'; import { startDaemonControlServer } from './controlServer'; @@ -69,6 +70,13 @@ export async function startDaemon(): Promise { requestShutdown('os-signal'); }); + if (isWindows()) { + process.on('SIGBREAK', () => { + logger.debug('[DAEMON RUN] Received SIGBREAK'); + requestShutdown('os-signal'); + }); + } + process.on('uncaughtException', (error) => { logger.debug('[DAEMON RUN] FATAL: Uncaught exception', error); logger.debug(`[DAEMON RUN] Stack trace: ${error.stack}`); @@ -362,16 +370,16 @@ export async function startDaemon(): Promise { if (session.startedBy === 'daemon' && session.childProcess) { try { - session.childProcess.kill('SIGTERM'); - logger.debug(`[DAEMON RUN] Sent SIGTERM to daemon-spawned session ${sessionId}`); + void killProcessByChildProcess(session.childProcess); + logger.debug(`[DAEMON RUN] Requested termination for daemon-spawned session ${sessionId}`); } catch (error) { logger.debug(`[DAEMON RUN] Failed to kill session ${sessionId}:`, error); } } else { // For externally started sessions, try to kill by PID try { - process.kill(pid, 'SIGTERM'); - logger.debug(`[DAEMON RUN] Sent SIGTERM to external session PID ${pid}`); + void killProcess(pid); + logger.debug(`[DAEMON RUN] Requested termination for external session PID ${pid}`); } catch (error) { logger.debug(`[DAEMON RUN] Failed to kill external session PID ${pid}:`, error); } @@ -467,11 +475,7 @@ export async function startDaemon(): Promise { // Prune stale sessions for (const [pid, _] of pidToTrackedSession.entries()) { - try { - // Check if process is still alive (signal 0 doesn't kill, just checks) - process.kill(pid, 0); - } catch (error) { - // Process is dead, remove from tracking + if (!isProcessAlive(pid)) { logger.debug(`[DAEMON RUN] Removing stale session with PID ${pid} (process no longer exists)`); pidToTrackedSession.delete(pid); } diff --git a/cli/src/daemon/uninstall.ts b/cli/src/daemon/uninstall.ts index 49467a18..e21ef05e 100644 --- a/cli/src/daemon/uninstall.ts +++ b/cli/src/daemon/uninstall.ts @@ -2,6 +2,10 @@ import { logger } from '@/ui/logger'; import { uninstall as uninstallMac } from './mac/uninstall'; export async function uninstall(): Promise { + if (process.platform === 'win32') { + throw new Error('Daemon uninstallation as Windows service not yet supported. Use "hapi daemon start".'); + } + if (process.platform !== 'darwin') { throw new Error('Daemon uninstallation is currently only supported on macOS'); } diff --git a/cli/src/persistence.ts b/cli/src/persistence.ts index 6e99c369..24d0e800 100644 --- a/cli/src/persistence.ts +++ b/cli/src/persistence.ts @@ -11,6 +11,7 @@ import { constants } from 'node:fs' import { configuration } from '@/configuration' import * as z from 'zod'; import { encodeBase64 } from '@/api/encryption'; +import { isProcessAlive } from '@/utils/process'; interface Settings { onboardingCompleted: boolean @@ -283,9 +284,7 @@ export async function acquireDaemonLock( try { const lockPid = readFileSync(configuration.daemonLockFile, 'utf-8').trim(); if (lockPid && !isNaN(Number(lockPid))) { - try { - process.kill(Number(lockPid), 0); // Check if process exists - } catch { + if (!isProcessAlive(Number(lockPid))) { // Process doesn't exist, remove stale lock unlinkSync(configuration.daemonLockFile); continue; // Retry acquisition diff --git a/cli/src/utils/process.ts b/cli/src/utils/process.ts new file mode 100644 index 00000000..9e800724 --- /dev/null +++ b/cli/src/utils/process.ts @@ -0,0 +1,71 @@ +import type { ChildProcess } from 'node:child_process'; +import spawn from 'cross-spawn'; + +export const isWindows = (): boolean => process.platform === 'win32'; + +export function isProcessAlive(pid: number): boolean { + if (!Number.isFinite(pid) || pid <= 0) { + return false; + } + + try { + process.kill(pid, 0); + return true; + } catch { + return false; + } +} + +function killProcessWindows(pid: number, force: boolean): boolean { + const args = ['/T', '/PID', pid.toString()]; + if (force) { + args.unshift('/F'); + } + try { + const result = spawn.sync('taskkill', args, { stdio: 'pipe' }); + if (result.error) { + return false; + } + return result.status === 0; + } catch { + return false; + } +} + +export async function killProcess(pid: number, force: boolean = false): Promise { + if (!Number.isFinite(pid) || pid <= 0) { + return false; + } + + if (isWindows()) { + return killProcessWindows(pid, force); + } + + try { + process.kill(pid, force ? 'SIGKILL' : 'SIGTERM'); + return true; + } catch { + return false; + } +} + +export async function killProcessByChildProcess( + child: ChildProcess, + force: boolean = false +): Promise { + const pid = child.pid; + if (!pid) { + return false; + } + + if (isWindows()) { + return killProcess(pid, force); + } + + try { + child.kill(force ? 'SIGKILL' : 'SIGTERM'); + return true; + } catch { + return false; + } +}