From d22deabe797481e9918b413f9c1ea51fa8a8226f Mon Sep 17 00:00:00 2001 From: Kacper Date: Fri, 16 Jan 2026 20:27:53 +0100 Subject: [PATCH 1/9] fix: improve process termination handling for Windows Updated the process termination logic in ClaudeUsageService to handle Windows environments correctly. The code now checks the operating system and calls the appropriate kill method, ensuring consistent behavior across platforms. --- apps/server/src/services/claude-usage-service.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/apps/server/src/services/claude-usage-service.ts b/apps/server/src/services/claude-usage-service.ts index 64dceb6a..59f22f20 100644 --- a/apps/server/src/services/claude-usage-service.ts +++ b/apps/server/src/services/claude-usage-service.ts @@ -277,9 +277,14 @@ export class ClaudeUsageService { ptyProcess.write('\x1b'); // Send escape key // Fallback: if ESC doesn't exit (Linux), use SIGTERM after 2s + // Windows doesn't support signals, so just call kill() without args setTimeout(() => { if (!settled && ptyProcess && !ptyProcess.killed) { - ptyProcess.kill('SIGTERM'); + if (this.isWindows) { + ptyProcess.kill(); + } else { + ptyProcess.kill('SIGTERM'); + } } }, 2000); } From f0e655f49ae5e90c6444a09fbbbbddc75b79e534 Mon Sep 17 00:00:00 2001 From: Kacper Date: Fri, 16 Jan 2026 20:34:12 +0100 Subject: [PATCH 2/9] fix: unify PTY process termination handling across platforms Refactored the process termination logic in both ClaudeUsageService and TerminalService to use a centralized method for killing PTY processes. This ensures consistent handling of process termination across Windows and Unix-like systems, improving reliability and maintainability of the code. --- .../src/services/claude-usage-service.ts | 28 +++++++++++++------ apps/server/src/services/terminal-service.ts | 25 +++++++++++++++-- 2 files changed, 42 insertions(+), 11 deletions(-) diff --git a/apps/server/src/services/claude-usage-service.ts b/apps/server/src/services/claude-usage-service.ts index 59f22f20..35c00a20 100644 --- a/apps/server/src/services/claude-usage-service.ts +++ b/apps/server/src/services/claude-usage-service.ts @@ -23,6 +23,22 @@ export class ClaudeUsageService { private isWindows = os.platform() === 'win32'; private isLinux = os.platform() === 'linux'; + /** + * 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 */ @@ -211,7 +227,7 @@ export class ClaudeUsageService { if (!settled) { settled = true; if (ptyProcess && !ptyProcess.killed) { - ptyProcess.kill(); + this.killPtyProcess(ptyProcess); } // Don't fail if we have data - return it instead if (output.includes('Current session')) { @@ -253,7 +269,7 @@ export class ClaudeUsageService { if (!settled) { settled = true; if (ptyProcess && !ptyProcess.killed) { - ptyProcess.kill(); + this.killPtyProcess(ptyProcess); } reject( new Error( @@ -277,14 +293,10 @@ export class ClaudeUsageService { ptyProcess.write('\x1b'); // Send escape key // Fallback: if ESC doesn't exit (Linux), use SIGTERM after 2s - // Windows doesn't support signals, so just call kill() without args + // Windows doesn't support signals, so killPtyProcess handles platform differences setTimeout(() => { if (!settled && ptyProcess && !ptyProcess.killed) { - if (this.isWindows) { - ptyProcess.kill(); - } else { - ptyProcess.kill('SIGTERM'); - } + this.killPtyProcess(ptyProcess); } }, 2000); } diff --git a/apps/server/src/services/terminal-service.ts b/apps/server/src/services/terminal-service.ts index c309975c..bd4481a8 100644 --- a/apps/server/src/services/terminal-service.ts +++ b/apps/server/src/services/terminal-service.ts @@ -70,6 +70,23 @@ export class TerminalService extends EventEmitter { private sessions: Map = new Map(); private dataCallbacks: Set = new Set(); private exitCallbacks: Set = new Set(); + private isWindows = os.platform() === 'win32'; + + /** + * Kill a PTY process with platform-specific handling. + * Windows doesn't support Unix signals like SIGTERM/SIGKILL, 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); + } + } /** * Detect the best shell for the current platform @@ -477,8 +494,9 @@ export class TerminalService extends EventEmitter { } // First try graceful SIGTERM to allow process cleanup + // On Windows, killPtyProcess calls kill() without signal since Windows doesn't support Unix signals logger.info(`Session ${sessionId} sending SIGTERM`); - session.pty.kill('SIGTERM'); + this.killPtyProcess(session.pty, 'SIGTERM'); // Schedule SIGKILL fallback if process doesn't exit gracefully // The onExit handler will remove session from map when it actually exits @@ -486,7 +504,7 @@ export class TerminalService extends EventEmitter { if (this.sessions.has(sessionId)) { logger.info(`Session ${sessionId} still alive after SIGTERM, sending SIGKILL`); try { - session.pty.kill('SIGKILL'); + this.killPtyProcess(session.pty, 'SIGKILL'); } catch { // Process may have already exited } @@ -588,7 +606,8 @@ export class TerminalService extends EventEmitter { if (session.flushTimeout) { clearTimeout(session.flushTimeout); } - session.pty.kill(); + // Use platform-specific kill to ensure proper termination on Windows + this.killPtyProcess(session.pty); } catch { // Ignore errors during cleanup } From d96f369b73467d4373b8dfae717d05b0e10a9240 Mon Sep 17 00:00:00 2001 From: Kacper Date: Fri, 16 Jan 2026 20:38:29 +0100 Subject: [PATCH 3/9] test: mock Unix platform for SIGTERM behavior in ClaudeUsageService tests Added a mock for the Unix platform in the SIGTERM test case to ensure proper behavior during testing on non-Windows systems. This change enhances the reliability of the tests by simulating the expected environment for process termination. --- .../server/tests/unit/services/claude-usage-service.test.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/apps/server/tests/unit/services/claude-usage-service.test.ts b/apps/server/tests/unit/services/claude-usage-service.test.ts index 4b3f3c94..65699ca6 100644 --- a/apps/server/tests/unit/services/claude-usage-service.test.ts +++ b/apps/server/tests/unit/services/claude-usage-service.test.ts @@ -681,7 +681,9 @@ Resets in 2h it('should send SIGTERM after ESC if process does not exit', async () => { vi.useFakeTimers(); - const windowsService = new ClaudeUsageService(); + // Mock Unix platform to test SIGTERM behavior (Windows calls kill() without signal) + vi.mocked(os.platform).mockReturnValue('darwin'); + const ptyService = new ClaudeUsageService(); let dataCallback: Function | undefined; @@ -696,7 +698,7 @@ Resets in 2h }; vi.mocked(pty.spawn).mockReturnValue(mockPty as any); - windowsService.fetchUsageData(); + ptyService.fetchUsageData(); // Simulate seeing usage data dataCallback!('Current session\n65% left'); From 0c452a3ebc651690fe391b0ae2aebb345886a9f7 Mon Sep 17 00:00:00 2001 From: Scott Date: Sun, 18 Jan 2026 12:59:46 -0700 Subject: [PATCH 4/9] fix: add cross-platform Node.js launcher for Windows CMD/PowerShell support The `./start-automaker.sh` script doesn't work when invoked from Windows CMD or PowerShell because: 1. The `./` prefix is Unix-style path notation 2. Windows shells don't execute .sh files directly This adds a Node.js launcher (`start-automaker.mjs`) that: - Detects the platform and finds bash (Git Bash, MSYS2, Cygwin, or WSL) - Converts Windows paths to Unix-style for bash compatibility - Passes all arguments through to the original bash script - Provides helpful error messages if bash isn't found The npm scripts now use `node start-automaker.mjs` which works on all platforms while preserving the full functionality of the bash TUI launcher. --- package.json | 4 +- start-automaker.mjs | 127 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 129 insertions(+), 2 deletions(-) create mode 100644 start-automaker.mjs diff --git a/package.json b/package.json index 1c884bc5..0756f868 100644 --- a/package.json +++ b/package.json @@ -12,8 +12,8 @@ "scripts": { "postinstall": "node -e \"const fs=require('fs');if(process.platform==='darwin'){['darwin-arm64','darwin-x64'].forEach(a=>{const p='node_modules/node-pty/prebuilds/'+a+'/spawn-helper';if(fs.existsSync(p))fs.chmodSync(p,0o755)})}\" && node scripts/fix-lockfile-urls.mjs", "fix:lockfile": "node scripts/fix-lockfile-urls.mjs", - "dev": "./start-automaker.sh", - "start": "./start-automaker.sh --production", + "dev": "node start-automaker.mjs", + "start": "node start-automaker.mjs --production", "_dev:web": "npm run dev:web --workspace=apps/ui", "_dev:electron": "npm run dev:electron --workspace=apps/ui", "_dev:electron:debug": "npm run dev:electron:debug --workspace=apps/ui", diff --git a/start-automaker.mjs b/start-automaker.mjs new file mode 100644 index 00000000..3106f792 --- /dev/null +++ b/start-automaker.mjs @@ -0,0 +1,127 @@ +#!/usr/bin/env node +/** + * Cross-platform launcher for Automaker + * Works on Windows (CMD, PowerShell, Git Bash) and Unix (macOS, Linux) + */ + +import { spawn } from 'child_process'; +import { existsSync } from 'fs'; +import { platform } from 'os'; +import { fileURLToPath } from 'url'; +import { dirname, join } from 'path'; + +const __filename = fileURLToPath(import.meta.url); +const __dirname = dirname(__filename); + +const isWindows = platform() === 'win32'; +const args = process.argv.slice(2); + +/** + * Find bash executable on Windows + */ +function findBashOnWindows() { + const possiblePaths = [ + // Git Bash (most common) + 'C:\\Program Files\\Git\\bin\\bash.exe', + 'C:\\Program Files (x86)\\Git\\bin\\bash.exe', + // MSYS2 + 'C:\\msys64\\usr\\bin\\bash.exe', + 'C:\\msys32\\usr\\bin\\bash.exe', + // Cygwin + 'C:\\cygwin64\\bin\\bash.exe', + 'C:\\cygwin\\bin\\bash.exe', + // WSL bash (available in PATH on Windows 10+) + 'bash.exe', + ]; + + for (const bashPath of possiblePaths) { + if (bashPath === 'bash.exe') { + // Check if bash is in PATH + try { + const result = spawn.sync?.('where', ['bash.exe'], { stdio: 'pipe' }); + if (result?.status === 0) { + return 'bash.exe'; + } + } catch { + // where command failed, continue checking + } + } else if (existsSync(bashPath)) { + return bashPath; + } + } + + return null; +} + +/** + * Run the bash script + */ +function runBashScript() { + const scriptPath = join(__dirname, 'start-automaker.sh'); + + if (!existsSync(scriptPath)) { + console.error('Error: start-automaker.sh not found'); + process.exit(1); + } + + let bashCmd; + let bashArgs; + + if (isWindows) { + bashCmd = findBashOnWindows(); + + if (!bashCmd) { + console.error('Error: Could not find bash on Windows.'); + console.error('Please install Git for Windows from https://git-scm.com/download/win'); + console.error(''); + console.error('Alternatively, you can run these commands directly:'); + console.error(' npm run dev:web - Web browser mode'); + console.error(' npm run dev:electron - Desktop app mode'); + process.exit(1); + } + + // Convert Windows path to Unix-style for bash + // Handle both C:\path and /c/path styles + let unixPath = scriptPath.replace(/\\/g, '/'); + if (/^[A-Za-z]:/.test(unixPath)) { + // Convert C:/path to /c/path for MSYS/Git Bash + unixPath = '/' + unixPath[0].toLowerCase() + unixPath.slice(2); + } + + bashArgs = [unixPath, ...args]; + } else { + bashCmd = '/bin/bash'; + bashArgs = [scriptPath, ...args]; + } + + const child = spawn(bashCmd, bashArgs, { + stdio: 'inherit', + env: { + ...process.env, + // Ensure proper terminal handling + TERM: process.env.TERM || 'xterm-256color', + }, + // On Windows, we need to use shell for proper signal handling + shell: false, + }); + + child.on('error', (err) => { + if (err.code === 'ENOENT') { + console.error(`Error: Could not find bash at "${bashCmd}"`); + console.error('Please ensure Git Bash or another bash shell is installed.'); + } else { + console.error('Error launching Automaker:', err.message); + } + process.exit(1); + }); + + child.on('exit', (code) => { + process.exit(code ?? 0); + }); + + // Forward signals to child process + process.on('SIGINT', () => child.kill('SIGINT')); + process.on('SIGTERM', () => child.kill('SIGTERM')); +} + +runBashScript(); From 8b5da3195bbaabd41519f061d34d01af5ecf1105 Mon Sep 17 00:00:00 2001 From: Scott Date: Sun, 18 Jan 2026 13:06:13 -0700 Subject: [PATCH 5/9] 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, }); From bfc23cdfa168f155a3cf265e67d1d4810b05ef78 Mon Sep 17 00:00:00 2001 From: Scott Date: Sun, 18 Jan 2026 13:12:11 -0700 Subject: [PATCH 6/9] fix: guard signal forwarding against race conditions --- start-automaker.mjs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/start-automaker.mjs b/start-automaker.mjs index 79ace0dd..04569819 100644 --- a/start-automaker.mjs +++ b/start-automaker.mjs @@ -119,9 +119,13 @@ function runBashScript() { process.exit(code ?? 0); }); - // Forward signals to child process - process.on('SIGINT', () => child.kill('SIGINT')); - process.on('SIGTERM', () => child.kill('SIGTERM')); + // Forward signals to child process (guard against race conditions) + process.on('SIGINT', () => { + if (!child.killed) child.kill('SIGINT'); + }); + process.on('SIGTERM', () => { + if (!child.killed) child.kill('SIGTERM'); + }); } runBashScript(); From e3213b142698fad7a5e80c6e51cd3b2c81cca886 Mon Sep 17 00:00:00 2001 From: Scott Date: Sun, 18 Jan 2026 13:30:04 -0700 Subject: [PATCH 7/9] fix: add WSL/Cygwin path translation and improve signal handling - Add convertPathForBash() function that detects bash variant: - Cygwin: /cygdrive/c/path - WSL: /mnt/c/path - MSYS/Git Bash: /c/path - Update exit handler to properly handle signal termination (exit code 1 when killed by signal vs code from child) Addresses remaining CodeRabbit PR #586 recommendations. Co-Authored-By: Claude Opus 4.5 --- start-automaker.mjs | 47 ++++++++++++++++++++++++++++++++++++--------- 1 file changed, 38 insertions(+), 9 deletions(-) diff --git a/start-automaker.mjs b/start-automaker.mjs index 04569819..4be58041 100644 --- a/start-automaker.mjs +++ b/start-automaker.mjs @@ -16,6 +16,36 @@ const __dirname = dirname(__filename); const isWindows = platform() === 'win32'; const args = process.argv.slice(2); +/** + * Convert Windows path to Unix-style for the detected bash variant + * @param {string} windowsPath - Windows-style path (e.g., C:\path\to\file) + * @param {string} bashCmd - Path to bash executable (used to detect variant) + * @returns {string} Unix-style path appropriate for the bash variant + */ +function convertPathForBash(windowsPath, bashCmd) { + let unixPath = windowsPath.replace(/\\/g, '/'); + if (/^[A-Za-z]:/.test(unixPath)) { + const drive = unixPath[0].toLowerCase(); + const pathPart = unixPath.slice(2); + + // Detect bash type from path + if (bashCmd.toLowerCase().includes('cygwin')) { + // Cygwin expects /cygdrive/c/path format + return `/cygdrive/${drive}${pathPart}`; + } else if ( + bashCmd.toLowerCase().includes('system32') || + bashCmd === 'bash.exe' + ) { + // WSL bash is typically in System32 or just 'bash.exe' in PATH + return `/mnt/${drive}${pathPart}`; + } else { + // MSYS2/Git Bash expects /c/path format + return `/${drive}${pathPart}`; + } + } + return unixPath; +} + /** * Find bash executable on Windows */ @@ -80,14 +110,8 @@ function runBashScript() { process.exit(1); } - // Convert Windows path to Unix-style for bash - // Handle both C:\path and /c/path styles - let unixPath = scriptPath.replace(/\\/g, '/'); - if (/^[A-Za-z]:/.test(unixPath)) { - // Convert C:/path to /c/path for MSYS/Git Bash - unixPath = '/' + unixPath[0].toLowerCase() + unixPath.slice(2); - } - + // Convert Windows path to appropriate Unix-style for the detected bash variant + const unixPath = convertPathForBash(scriptPath, bashCmd); bashArgs = [unixPath, ...args]; } else { bashCmd = '/bin/bash'; @@ -115,7 +139,12 @@ function runBashScript() { process.exit(1); }); - child.on('exit', (code) => { + child.on('exit', (code, signal) => { + if (signal) { + // Process was killed by a signal - exit with 1 to indicate abnormal termination + // (Unix convention is 128 + signal number, but we use 1 for simplicity) + process.exit(1); + } process.exit(code ?? 0); }); From b91d84ee844dc7455df9e9873de31d6c698eb86f Mon Sep 17 00:00:00 2001 From: Scott Date: Sun, 18 Jan 2026 13:39:15 -0700 Subject: [PATCH 8/9] fix: improve bash detection and add input validation - Add detectBashVariant() that checks $OSTYPE for reliable WSL/MSYS/Cygwin detection instead of relying solely on executable path - Add input validation to convertPathForBash() to catch null/undefined args - Add validate_port() function in bash script to reject invalid port input (non-numeric, out of range) with clear error messages Co-Authored-By: Claude Opus 4.5 --- start-automaker.mjs | 67 ++++++++++++++++++++++++++++++++++++--------- start-automaker.sh | 54 +++++++++++++++++++++++++++++++++--- 2 files changed, 104 insertions(+), 17 deletions(-) diff --git a/start-automaker.mjs b/start-automaker.mjs index 4be58041..97362312 100644 --- a/start-automaker.mjs +++ b/start-automaker.mjs @@ -16,6 +16,39 @@ const __dirname = dirname(__filename); const isWindows = platform() === 'win32'; const args = process.argv.slice(2); +/** + * Detect the bash variant by checking $OSTYPE + * This is more reliable than path-based detection since bash.exe in PATH + * could be Git Bash, WSL, or something else + * @param {string} bashPath - Path to bash executable + * @returns {'WSL' | 'MSYS' | 'CYGWIN' | 'UNKNOWN'} The detected bash variant + */ +function detectBashVariant(bashPath) { + try { + const result = spawnSync(bashPath, ['-c', 'echo $OSTYPE'], { + stdio: 'pipe', + timeout: 2000, + }); + if (result.status === 0) { + const ostype = result.stdout.toString().trim(); + // WSL reports 'linux-gnu' or similar Linux identifier + if (ostype === 'linux-gnu' || ostype.startsWith('linux')) return 'WSL'; + // MSYS2/Git Bash reports 'msys' or 'mingw*' + if (ostype.startsWith('msys') || ostype.startsWith('mingw')) return 'MSYS'; + // Cygwin reports 'cygwin' + if (ostype.startsWith('cygwin')) return 'CYGWIN'; + } + } catch { + // Fall through to path-based detection + } + // Fallback to path-based detection if $OSTYPE check fails + const lower = bashPath.toLowerCase(); + if (lower.includes('cygwin')) return 'CYGWIN'; + if (lower.includes('system32')) return 'WSL'; + // Default to MSYS (Git Bash) as it's the most common + return 'MSYS'; +} + /** * Convert Windows path to Unix-style for the detected bash variant * @param {string} windowsPath - Windows-style path (e.g., C:\path\to\file) @@ -23,24 +56,32 @@ const args = process.argv.slice(2); * @returns {string} Unix-style path appropriate for the bash variant */ function convertPathForBash(windowsPath, bashCmd) { + // Input validation + if (!windowsPath || typeof windowsPath !== 'string') { + throw new Error('convertPathForBash: invalid windowsPath'); + } + if (!bashCmd || typeof bashCmd !== 'string') { + throw new Error('convertPathForBash: invalid bashCmd'); + } + let unixPath = windowsPath.replace(/\\/g, '/'); if (/^[A-Za-z]:/.test(unixPath)) { const drive = unixPath[0].toLowerCase(); const pathPart = unixPath.slice(2); - // Detect bash type from path - if (bashCmd.toLowerCase().includes('cygwin')) { - // Cygwin expects /cygdrive/c/path format - return `/cygdrive/${drive}${pathPart}`; - } else if ( - bashCmd.toLowerCase().includes('system32') || - bashCmd === 'bash.exe' - ) { - // WSL bash is typically in System32 or just 'bash.exe' in PATH - return `/mnt/${drive}${pathPart}`; - } else { - // MSYS2/Git Bash expects /c/path format - return `/${drive}${pathPart}`; + // Detect bash variant via $OSTYPE (more reliable than path-based) + const variant = detectBashVariant(bashCmd); + switch (variant) { + case 'CYGWIN': + // Cygwin expects /cygdrive/c/path format + return `/cygdrive/${drive}${pathPart}`; + case 'WSL': + // WSL expects /mnt/c/path format + return `/mnt/${drive}${pathPart}`; + case 'MSYS': + default: + // MSYS2/Git Bash expects /c/path format + return `/${drive}${pathPart}`; } } return unixPath; diff --git a/start-automaker.sh b/start-automaker.sh index 94f143ab..62adb4a4 100755 --- a/start-automaker.sh +++ b/start-automaker.sh @@ -37,6 +37,32 @@ DEFAULT_SERVER_PORT=3008 WEB_PORT=$DEFAULT_WEB_PORT SERVER_PORT=$DEFAULT_SERVER_PORT +# Port validation function +# Returns 0 if valid, 1 if invalid (with error message printed) +validate_port() { + local port="$1" + local port_name="${2:-port}" + + # Check if port is a number + if ! [[ "$port" =~ ^[0-9]+$ ]]; then + echo "${C_RED}Error:${RESET} $port_name must be a number, got '$port'" + return 1 + fi + + # Check if port is in valid range (1-65535) + if [ "$port" -lt 1 ] || [ "$port" -gt 65535 ]; then + echo "${C_RED}Error:${RESET} $port_name must be between 1-65535, got '$port'" + return 1 + fi + + # Check if port is in privileged range (warning only) + if [ "$port" -lt 1024 ]; then + echo "${C_YELLOW}Warning:${RESET} $port_name $port is in privileged range (requires root/admin)" + fi + + return 0 +} + # Hostname configuration # Use VITE_HOSTNAME if explicitly set, otherwise default to localhost # Note: Don't use $HOSTNAME as it's a bash built-in containing the machine's hostname @@ -510,9 +536,19 @@ check_ports() { ;; [uU]|[uU][sS][eE]) read -r -p "Enter web port (default $DEFAULT_WEB_PORT): " input_web - WEB_PORT=${input_web:-$DEFAULT_WEB_PORT} + input_web=${input_web:-$DEFAULT_WEB_PORT} + if ! validate_port "$input_web" "Web port"; then + continue + fi + WEB_PORT=$input_web + read -r -p "Enter server port (default $DEFAULT_SERVER_PORT): " input_server - SERVER_PORT=${input_server:-$DEFAULT_SERVER_PORT} + input_server=${input_server:-$DEFAULT_SERVER_PORT} + if ! validate_port "$input_server" "Server port"; then + continue + fi + SERVER_PORT=$input_server + echo "${C_GREEN}Using ports: Web=$WEB_PORT, Server=$SERVER_PORT${RESET}" break ;; @@ -802,10 +838,20 @@ resolve_port_conflicts() { local input_pad=$(( (TERM_COLS - 40) / 2 )) printf "%${input_pad}s" "" read -r -p "Enter web port (default $DEFAULT_WEB_PORT): " input_web - WEB_PORT=${input_web:-$DEFAULT_WEB_PORT} + input_web=${input_web:-$DEFAULT_WEB_PORT} + if ! validate_port "$input_web" "Web port"; then + continue + fi + WEB_PORT=$input_web + printf "%${input_pad}s" "" read -r -p "Enter server port (default $DEFAULT_SERVER_PORT): " input_server - SERVER_PORT=${input_server:-$DEFAULT_SERVER_PORT} + input_server=${input_server:-$DEFAULT_SERVER_PORT} + if ! validate_port "$input_server" "Server port"; then + continue + fi + SERVER_PORT=$input_server + center_print "Using ports: Web=$WEB_PORT, Server=$SERVER_PORT" "$C_GREEN" break ;; From 2854e24e8401cdd38a0e1b9ca2f5d4f22b97f7a4 Mon Sep 17 00:00:00 2001 From: Scott Date: Sun, 18 Jan 2026 13:46:11 -0700 Subject: [PATCH 9/9] fix: validate both ports before assigning either Collect web and server port inputs first, then validate both before assigning to global variables. This prevents WEB_PORT from being modified when SERVER_PORT validation subsequently fails. Co-Authored-By: Claude Opus 4.5 --- start-automaker.sh | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/start-automaker.sh b/start-automaker.sh index 62adb4a4..ee273348 100755 --- a/start-automaker.sh +++ b/start-automaker.sh @@ -535,20 +535,23 @@ check_ports() { break ;; [uU]|[uU][sS][eE]) + # Collect both ports first read -r -p "Enter web port (default $DEFAULT_WEB_PORT): " input_web input_web=${input_web:-$DEFAULT_WEB_PORT} + read -r -p "Enter server port (default $DEFAULT_SERVER_PORT): " input_server + input_server=${input_server:-$DEFAULT_SERVER_PORT} + + # Validate both before assigning either if ! validate_port "$input_web" "Web port"; then continue fi - WEB_PORT=$input_web - - read -r -p "Enter server port (default $DEFAULT_SERVER_PORT): " input_server - input_server=${input_server:-$DEFAULT_SERVER_PORT} if ! validate_port "$input_server" "Server port"; then continue fi - SERVER_PORT=$input_server + # Assign atomically after both validated + WEB_PORT=$input_web + SERVER_PORT=$input_server echo "${C_GREEN}Using ports: Web=$WEB_PORT, Server=$SERVER_PORT${RESET}" break ;; @@ -836,22 +839,25 @@ resolve_port_conflicts() { [uU]|[uU][sS][eE]) echo "" local input_pad=$(( (TERM_COLS - 40) / 2 )) + # Collect both ports first printf "%${input_pad}s" "" read -r -p "Enter web port (default $DEFAULT_WEB_PORT): " input_web input_web=${input_web:-$DEFAULT_WEB_PORT} - if ! validate_port "$input_web" "Web port"; then - continue - fi - WEB_PORT=$input_web - printf "%${input_pad}s" "" read -r -p "Enter server port (default $DEFAULT_SERVER_PORT): " input_server input_server=${input_server:-$DEFAULT_SERVER_PORT} + + # Validate both before assigning either + if ! validate_port "$input_web" "Web port"; then + continue + fi if ! validate_port "$input_server" "Server port"; then continue fi - SERVER_PORT=$input_server + # Assign atomically after both validated + WEB_PORT=$input_web + SERVER_PORT=$input_server center_print "Using ports: Web=$WEB_PORT, Server=$SERVER_PORT" "$C_GREEN" break ;;