From 086bf958e9c6c9d6d745e78a29a95a9a58490bf1 Mon Sep 17 00:00:00 2001 From: Tam Nhu Tran Date: Wed, 11 Feb 2026 01:47:54 +0700 Subject: [PATCH 1/4] fix(cliproxy): prevent OAuth process hang on Qwen device code flow - Escalate SIGTERM to SIGKILL after 3s if Go binary is stuck in blocking HTTP call - Extend timeout to 300s for device code flows matching CLIProxy binary polling window - Apply SIGKILL escalation to both SIGINT cleanup and timeout handlers Closes #314 --- src/cliproxy/auth/oauth-process.ts | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/src/cliproxy/auth/oauth-process.ts b/src/cliproxy/auth/oauth-process.ts index 9f6fb39d..344e156b 100644 --- a/src/cliproxy/auth/oauth-process.ts +++ b/src/cliproxy/auth/oauth-process.ts @@ -6,6 +6,19 @@ */ import { spawn, ChildProcess } from 'child_process'; + +/** + * Kill process with SIGTERM, escalating to SIGKILL if it doesn't exit + */ +function killWithEscalation(proc: ChildProcess, gracePeriodMs = 3000): void { + proc.kill('SIGTERM'); + const timer = setTimeout(() => { + if (proc.exitCode === null) { + proc.kill('SIGKILL'); + } + }, gracePeriodMs); + proc.once('exit', () => clearTimeout(timer)); +} import { ok, fail, info, warn } from '../../utils/ui'; import { tryKiroImport } from './kiro-import'; import { CLIProxyProvider } from '../types'; @@ -333,8 +346,8 @@ export function executeOAuthProcess(options: OAuthProcessOptions): Promise { if (stdinKeepalive) clearInterval(stdinKeepalive); - if (authProcess && !authProcess.killed) { - authProcess.kill('SIGTERM'); + if (authProcess && authProcess.exitCode === null) { + killWithEscalation(authProcess); } }; process.on('SIGINT', cleanup); @@ -425,7 +438,8 @@ export function executeOAuthProcess(options: OAuthProcessOptions): Promise { // H7: Clear stdin keepalive interval if (stdinKeepalive) clearInterval(stdinKeepalive); @@ -435,9 +449,9 @@ export function executeOAuthProcess(options: OAuthProcessOptions): Promise Date: Wed, 11 Feb 2026 01:58:35 +0700 Subject: [PATCH 2/4] fix(delegation): use exitCode instead of killed for process termination checks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Replace `!proc.killed` with `proc.exitCode === null` in headless-executor - Extract `killWithEscalation` helper for SIGTERM→SIGKILL escalation - Fixes pre-existing dead code where SIGKILL was never sent --- src/delegation/headless-executor.ts | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/src/delegation/headless-executor.ts b/src/delegation/headless-executor.ts index 1a8be770..3f803c03 100644 --- a/src/delegation/headless-executor.ts +++ b/src/delegation/headless-executor.ts @@ -5,7 +5,20 @@ * Spawns claude with -p flag for single-turn execution */ -import { spawn } from 'child_process'; +import { spawn, ChildProcess } from 'child_process'; + +/** + * Kill process with SIGTERM, escalating to SIGKILL if it doesn't exit + */ +function killWithEscalation(proc: ChildProcess, gracePeriodMs = 3000): void { + proc.kill('SIGTERM'); + const timer = setTimeout(() => { + if (proc.exitCode === null) { + proc.kill('SIGKILL'); + } + }, gracePeriodMs); + proc.once('exit', () => clearTimeout(timer)); +} import * as path from 'path'; import * as fs from 'fs'; import { SessionManager } from './session-manager'; @@ -214,11 +227,8 @@ export class HeadlessExecutor { // Setup signal handlers for cleanup const cleanupHandler = () => { - if (!proc.killed) { - proc.kill('SIGTERM'); - setTimeout(() => { - if (!proc.killed) proc.kill('SIGKILL'); - }, 2000); + if (proc.exitCode === null) { + killWithEscalation(proc, 2000); } }; process.once('SIGINT', cleanupHandler); @@ -326,16 +336,13 @@ export class HeadlessExecutor { // Handle timeout if (timeout > 0) { const timeoutHandle = setTimeout(() => { - if (!proc.killed) { + if (proc.exitCode === null) { timedOut = true; if (progressInterval) { clearInterval(progressInterval); process.stderr.write('\r\x1b[K'); } - proc.kill('SIGTERM'); - setTimeout(() => { - if (!proc.killed) proc.kill('SIGKILL'); - }, 10000); + killWithEscalation(proc, 10000); } }, timeout); proc.on('close', () => clearTimeout(timeoutHandle)); From 90b4627740ae90374b06e0013c09bd583e1ddb39 Mon Sep 17 00:00:00 2001 From: Tam Nhu Tran Date: Wed, 11 Feb 2026 02:08:12 +0700 Subject: [PATCH 3/4] refactor(utils): extract killWithEscalation to shared process-utils - Move killWithEscalation to src/utils/process-utils.ts (DRY) - Fix handleCancel to use exitCode === null and killWithEscalation - Fix import ordering in both oauth-process.ts and headless-executor.ts --- src/cliproxy/auth/oauth-process.ts | 18 +++--------------- src/delegation/headless-executor.ts | 16 ++-------------- src/utils/process-utils.ts | 20 ++++++++++++++++++++ 3 files changed, 25 insertions(+), 29 deletions(-) create mode 100644 src/utils/process-utils.ts diff --git a/src/cliproxy/auth/oauth-process.ts b/src/cliproxy/auth/oauth-process.ts index 344e156b..ba157784 100644 --- a/src/cliproxy/auth/oauth-process.ts +++ b/src/cliproxy/auth/oauth-process.ts @@ -6,20 +6,8 @@ */ import { spawn, ChildProcess } from 'child_process'; - -/** - * Kill process with SIGTERM, escalating to SIGKILL if it doesn't exit - */ -function killWithEscalation(proc: ChildProcess, gracePeriodMs = 3000): void { - proc.kill('SIGTERM'); - const timer = setTimeout(() => { - if (proc.exitCode === null) { - proc.kill('SIGKILL'); - } - }, gracePeriodMs); - proc.once('exit', () => clearTimeout(timer)); -} import { ok, fail, info, warn } from '../../utils/ui'; +import { killWithEscalation } from '../../utils/process-utils'; import { tryKiroImport } from './kiro-import'; import { CLIProxyProvider } from '../types'; import { AccountInfo } from '../account-manager'; @@ -371,9 +359,9 @@ export function executeOAuthProcess(options: OAuthProcessOptions): Promise { - if (cancelledSessionId === state.sessionId && authProcess && !authProcess.killed) { + if (cancelledSessionId === state.sessionId && authProcess && authProcess.exitCode === null) { log('Session cancelled externally'); - authProcess.kill('SIGTERM'); + killWithEscalation(authProcess); } }; authSessionEvents.on('session:cancelled', handleCancel); diff --git a/src/delegation/headless-executor.ts b/src/delegation/headless-executor.ts index 3f803c03..977b5ff7 100644 --- a/src/delegation/headless-executor.ts +++ b/src/delegation/headless-executor.ts @@ -5,21 +5,9 @@ * Spawns claude with -p flag for single-turn execution */ -import { spawn, ChildProcess } from 'child_process'; - -/** - * Kill process with SIGTERM, escalating to SIGKILL if it doesn't exit - */ -function killWithEscalation(proc: ChildProcess, gracePeriodMs = 3000): void { - proc.kill('SIGTERM'); - const timer = setTimeout(() => { - if (proc.exitCode === null) { - proc.kill('SIGKILL'); - } - }, gracePeriodMs); - proc.once('exit', () => clearTimeout(timer)); -} +import { spawn } from 'child_process'; import * as path from 'path'; +import { killWithEscalation } from '../utils/process-utils'; import * as fs from 'fs'; import { SessionManager } from './session-manager'; import { SettingsParser } from './settings-parser'; diff --git a/src/utils/process-utils.ts b/src/utils/process-utils.ts new file mode 100644 index 00000000..ac22f8ba --- /dev/null +++ b/src/utils/process-utils.ts @@ -0,0 +1,20 @@ +/** + * Process management utilities + */ + +import { ChildProcess } from 'child_process'; + +/** + * Kill process with SIGTERM, escalating to SIGKILL if it doesn't exit. + * Uses exitCode === null (not proc.killed) to check if process is still running, + * since proc.killed only indicates a signal was sent, not that the process exited. + */ +export function killWithEscalation(proc: ChildProcess, gracePeriodMs = 3000): void { + proc.kill('SIGTERM'); + const timer = setTimeout(() => { + if (proc.exitCode === null) { + proc.kill('SIGKILL'); + } + }, gracePeriodMs); + proc.once('exit', () => clearTimeout(timer)); +} From dc9b27623bd3cd92dd1b556329d20fd62043b37b Mon Sep 17 00:00:00 2001 From: Tam Nhu Tran Date: Wed, 11 Feb 2026 02:25:10 +0700 Subject: [PATCH 4/4] test(utils): add unit tests for killWithEscalation - Add 7 test cases covering SIGTERM/SIGKILL escalation, timer cleanup, default and custom grace periods, and already-exited process edge case - Add timer.unref() to prevent keeping event loop alive during shutdown - Add comment explaining 10s grace period in headless-executor timeout --- src/delegation/headless-executor.ts | 1 + src/utils/process-utils.ts | 1 + tests/unit/utils/process-utils.test.ts | 132 +++++++++++++++++++++++++ 3 files changed, 134 insertions(+) create mode 100644 tests/unit/utils/process-utils.test.ts diff --git a/src/delegation/headless-executor.ts b/src/delegation/headless-executor.ts index 977b5ff7..da95e746 100644 --- a/src/delegation/headless-executor.ts +++ b/src/delegation/headless-executor.ts @@ -330,6 +330,7 @@ export class HeadlessExecutor { clearInterval(progressInterval); process.stderr.write('\r\x1b[K'); } + // Longer grace period for timeout (vs 2s for SIGINT) since delegated sessions may need time to flush output killWithEscalation(proc, 10000); } }, timeout); diff --git a/src/utils/process-utils.ts b/src/utils/process-utils.ts index ac22f8ba..571ce743 100644 --- a/src/utils/process-utils.ts +++ b/src/utils/process-utils.ts @@ -16,5 +16,6 @@ export function killWithEscalation(proc: ChildProcess, gracePeriodMs = 3000): vo proc.kill('SIGKILL'); } }, gracePeriodMs); + timer.unref(); // Don't keep event loop alive just for escalation proc.once('exit', () => clearTimeout(timer)); } diff --git a/tests/unit/utils/process-utils.test.ts b/tests/unit/utils/process-utils.test.ts new file mode 100644 index 00000000..cfae232a --- /dev/null +++ b/tests/unit/utils/process-utils.test.ts @@ -0,0 +1,132 @@ +/** + * Unit tests for process-utils.ts + */ +import { describe, it, expect, beforeEach, afterEach, jest } from 'bun:test'; +import { EventEmitter } from 'events'; +import { killWithEscalation } from '../../../src/utils/process-utils'; +import type { ChildProcess } from 'child_process'; + +// Mock ChildProcess using EventEmitter +function createMockProcess(exitCode: number | null = null): ChildProcess { + const proc = new EventEmitter() as any; + proc.killed = false; + proc.exitCode = exitCode; + proc.kill = jest.fn((signal?: string) => { + if (signal === 'SIGTERM' || signal === 'SIGKILL') { + proc.killed = true; + } + return true; + }); + return proc as ChildProcess; +} + +describe('killWithEscalation', () => { + beforeEach(() => { + jest.useFakeTimers(); + }); + + afterEach(() => { + jest.useRealTimers(); + }); + + it('should send SIGTERM immediately', () => { + const proc = createMockProcess(); + killWithEscalation(proc); + + expect(proc.kill).toHaveBeenCalledWith('SIGTERM'); + expect(proc.kill).toHaveBeenCalledTimes(1); + }); + + it('should send SIGKILL after grace period if process still running', () => { + const proc = createMockProcess(null); // exitCode null = still running + killWithEscalation(proc, 3000); + + // SIGTERM sent immediately + expect(proc.kill).toHaveBeenCalledWith('SIGTERM'); + expect(proc.kill).toHaveBeenCalledTimes(1); + + // Advance time by grace period + jest.advanceTimersByTime(3000); + + // SIGKILL sent after grace period + expect(proc.kill).toHaveBeenCalledWith('SIGKILL'); + expect(proc.kill).toHaveBeenCalledTimes(2); + }); + + it('should NOT send SIGKILL if process exits before grace period', () => { + const proc = createMockProcess(null); + killWithEscalation(proc, 3000); + + expect(proc.kill).toHaveBeenCalledWith('SIGTERM'); + expect(proc.kill).toHaveBeenCalledTimes(1); + + // Simulate process exit after 1 second + jest.advanceTimersByTime(1000); + proc.exitCode = 0; // Process exited + proc.emit('exit', 0); + + // Advance remaining time + jest.advanceTimersByTime(2000); + + // SIGKILL should NOT have been sent + expect(proc.kill).toHaveBeenCalledTimes(1); + expect(proc.kill).not.toHaveBeenCalledWith('SIGKILL'); + }); + + it('should use default grace period of 3000ms', () => { + const proc = createMockProcess(null); + killWithEscalation(proc); // No grace period argument + + expect(proc.kill).toHaveBeenCalledWith('SIGTERM'); + + // Advance by default 3000ms + jest.advanceTimersByTime(3000); + + expect(proc.kill).toHaveBeenCalledWith('SIGKILL'); + }); + + it('should respect custom grace period', () => { + const proc = createMockProcess(null); + killWithEscalation(proc, 5000); // Custom 5 second grace period + + expect(proc.kill).toHaveBeenCalledWith('SIGTERM'); + + // Advance by less than grace period + jest.advanceTimersByTime(4999); + expect(proc.kill).toHaveBeenCalledTimes(1); // Still only SIGTERM + + // Advance to grace period + jest.advanceTimersByTime(1); + expect(proc.kill).toHaveBeenCalledWith('SIGKILL'); + }); + + it('should clear timer when process exits', () => { + const proc = createMockProcess(null); + killWithEscalation(proc, 3000); + + // Simulate immediate exit + proc.exitCode = 0; + proc.emit('exit', 0); + + // Advance way past grace period + jest.advanceTimersByTime(10000); + + // Should only have SIGTERM, timer was cleared + expect(proc.kill).toHaveBeenCalledTimes(1); + expect(proc.kill).toHaveBeenCalledWith('SIGTERM'); + }); + + it('should handle process that already exited', () => { + const proc = createMockProcess(0); // Already exited + killWithEscalation(proc, 3000); + + expect(proc.kill).toHaveBeenCalledWith('SIGTERM'); + + // Even though exitCode is not null, timer still fires + // (because we check exitCode at timer callback time) + jest.advanceTimersByTime(3000); + + // SIGKILL should NOT be sent because exitCode is not null + expect(proc.kill).toHaveBeenCalledTimes(1); + }); +});