From 8b5da3195bbaabd41519f061d34d01af5ecf1105 Mon Sep 17 00:00:00 2001 From: Scott Date: Sun, 18 Jan 2026 13:06:13 -0700 Subject: [PATCH] fix: address PR review feedback - Remove duplicate killPtyProcess method in claude-usage-service.ts - Import and use spawnSync correctly instead of spawn.sync - Fix misleading comment about shell option and signal handling --- apps/server/src/services/claude-usage-service.ts | 16 ---------------- start-automaker.mjs | 10 +++++----- 2 files changed, 5 insertions(+), 21 deletions(-) diff --git a/apps/server/src/services/claude-usage-service.ts b/apps/server/src/services/claude-usage-service.ts index 9c2f8b78..aebed98b 100644 --- a/apps/server/src/services/claude-usage-service.ts +++ b/apps/server/src/services/claude-usage-service.ts @@ -46,22 +46,6 @@ export class ClaudeUsageService { } } - /** - * Kill a PTY process with platform-specific handling. - * Windows doesn't support Unix signals like SIGTERM, so we call kill() without arguments. - * On Unix-like systems (macOS, Linux), we can specify the signal. - * - * @param ptyProcess - The PTY process to kill - * @param signal - The signal to send on Unix-like systems (default: 'SIGTERM') - */ - private killPtyProcess(ptyProcess: pty.IPty, signal: string = 'SIGTERM'): void { - if (this.isWindows) { - ptyProcess.kill(); - } else { - ptyProcess.kill(signal); - } - } - /** * Check if Claude CLI is available on the system */ diff --git a/start-automaker.mjs b/start-automaker.mjs index 3106f792..79ace0dd 100644 --- a/start-automaker.mjs +++ b/start-automaker.mjs @@ -4,7 +4,7 @@ * Works on Windows (CMD, PowerShell, Git Bash) and Unix (macOS, Linux) */ -import { spawn } from 'child_process'; +import { spawn, spawnSync } from 'child_process'; import { existsSync } from 'fs'; import { platform } from 'os'; import { fileURLToPath } from 'url'; @@ -38,12 +38,12 @@ function findBashOnWindows() { if (bashPath === 'bash.exe') { // Check if bash is in PATH try { - const result = spawn.sync?.('where', ['bash.exe'], { stdio: 'pipe' }); + const result = spawnSync('where', ['bash.exe'], { stdio: 'pipe' }); if (result?.status === 0) { return 'bash.exe'; } - } catch { - // where command failed, continue checking + } catch (err) { + // where command failed, continue checking other paths } } else if (existsSync(bashPath)) { return bashPath; @@ -101,7 +101,7 @@ function runBashScript() { // Ensure proper terminal handling TERM: process.env.TERM || 'xterm-256color', }, - // On Windows, we need to use shell for proper signal handling + // shell: false ensures signals are forwarded directly to the child process shell: false, });