ros/automaker
1
0
Fork 0
mirror of https://github.com/AutoMaker-Org/automaker.git synced 2026-01-30 06:12:03 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: unify PTY process termination handling across platforms

Browse Source 
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.
This commit is contained in:
Kacper
2026-01-16 20:34:12 +01:00
parent be63a59e9c
commit 0e9369816f
2 changed files with 42 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files

28
apps/server/src/services/claude-usage-service.ts
View File

@@ -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);
}

25
apps/server/src/services/terminal-service.ts
View File

@@ -70,6 +70,23 @@ export class TerminalService extends EventEmitter {
private sessions: Map<string, TerminalSession> = new Map();
private dataCallbacks: Set<DataCallback> = new Set();
private exitCallbacks: Set<ExitCallback> = 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
}