From b3499bb5edef3c05fddf5bdd5ebe60cbd3221f3a Mon Sep 17 00:00:00 2001 From: weishu Date: Fri, 26 Dec 2025 17:31:05 +0800 Subject: [PATCH] fix: terminal state restoration and abort handling regression MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes regression introduced in 1a23bfa43026f15de68d6b57465fb00607fefe53. - Add restoreTerminalState helper to disable kitty keyboard protocol and CSI u key release reporting when switching modes or cleaning up - Improve claudeLocal abort handling with SIGINT → SIGTERM → SIGKILL escalation timeouts for graceful process termination - Fix useSwitchControls to properly handle CSI u space sequences (0x20) and key.name/key.sequence detection for space key presses - Fix typo: exutFuture → exitFuture in claudeLocalLauncher --- cli/src/claude/claudeLocal.ts | 93 +++++++++++++++++++++++- cli/src/claude/claudeLocalLauncher.ts | 6 +- cli/src/claude/claudeRemoteLauncher.ts | 6 +- cli/src/claude/runClaude.ts | 2 + cli/src/ui/ink/useSwitchControls.test.ts | 49 +++++++++++++ cli/src/ui/ink/useSwitchControls.ts | 10 ++- 6 files changed, 158 insertions(+), 8 deletions(-) diff --git a/cli/src/claude/claudeLocal.ts b/cli/src/claude/claudeLocal.ts index 63db54d9..2c860081 100644 --- a/cli/src/claude/claudeLocal.ts +++ b/cli/src/claude/claudeLocal.ts @@ -1,11 +1,20 @@ import { spawn } from "node:child_process"; import { mkdirSync } from "node:fs"; import { logger } from "@/ui/logger"; +import { restoreTerminalState } from "@/ui/terminalState"; import { claudeCheckSession } from "./utils/claudeCheckSession"; import { getProjectPath } from "./utils/path"; import { systemPrompt } from "./utils/systemPrompt"; import { withBunRuntimeEnv } from "@/utils/bunRuntime"; +const isAbortError = (error: unknown): boolean => { + if (!error || typeof error !== 'object') { + return false; + } + const maybeError = error as { name?: string; code?: string }; + return maybeError.name === 'AbortError' || maybeError.code === 'ABORT_ERR'; +}; + export async function claudeLocal(opts: { abort: AbortSignal, sessionId: string | null, @@ -34,6 +43,11 @@ export async function claudeLocal(opts: { startFrom = null; } + if (opts.abort.aborted) { + logger.debug('[ClaudeLocal] Abort already signaled before spawn; skipping launch'); + return startFrom ?? null; + } + // Spawn the process try { // Start the interactive process @@ -79,16 +93,84 @@ export async function claudeLocal(opts: { const child = spawn('claude', args, { stdio: ['inherit', 'inherit', 'inherit'], signal: opts.abort, + killSignal: 'SIGINT', cwd: opts.path, env: withBunRuntimeEnv(env, { allowBunBeBun: false }), shell: process.platform === 'win32' }); let settled = false; + const abortTimeoutMs = { + term: 1000, + kill: 3000 + }; + let abortTermTimeout: NodeJS.Timeout | null = null; + let abortKillTimeout: NodeJS.Timeout | null = null; + let forcedTermination = false; + let abortStartedAt: number | null = null; + + const isAlive = () => child.exitCode === null && !child.killed; + const formatAbortElapsed = () => { + if (abortStartedAt === null) { + return 'n/a'; + } + return `${Date.now() - abortStartedAt}ms`; + }; + + const abortHandler = () => { + if (abortTermTimeout || abortKillTimeout) { + logger.debug('[ClaudeLocal] Abort already in progress'); + return; + } + abortStartedAt = Date.now(); + logger.debug('[ClaudeLocal] Abort signaled, waiting for SIGINT to exit'); + abortTermTimeout = setTimeout(() => { + if (isAlive()) { + forcedTermination = true; + logger.debug(`[ClaudeLocal] Abort timeout reached (${formatAbortElapsed()}), sending SIGTERM`); + try { + child.kill('SIGTERM'); + } catch (error) { + logger.debug('[ClaudeLocal] Failed to send SIGTERM', error); + } + } + + abortKillTimeout = setTimeout(() => { + if (isAlive()) { + forcedTermination = true; + logger.debug(`[ClaudeLocal] Abort timeout reached (${formatAbortElapsed()}), sending SIGKILL`); + try { + child.kill('SIGKILL'); + } catch (error) { + logger.debug('[ClaudeLocal] Failed to send SIGKILL', error); + } + } + }, abortTimeoutMs.kill); + }, abortTimeoutMs.term); + }; + + if (opts.abort.aborted) { + abortHandler(); + } else { + opts.abort.addEventListener('abort', abortHandler); + } + + const cleanupAbortHandler = () => { + if (abortTermTimeout) { + clearTimeout(abortTermTimeout); + abortTermTimeout = null; + } + if (abortKillTimeout) { + clearTimeout(abortKillTimeout); + abortKillTimeout = null; + } + opts.abort.removeEventListener('abort', abortHandler); + }; const finalize = (error?: Error) => { if (settled) { return; } settled = true; + cleanupAbortHandler(); if (error) { reject(error); } else { @@ -96,11 +178,19 @@ export async function claudeLocal(opts: { } }; child.on('error', (error) => { + if (opts.abort.aborted || isAbortError(error)) { + logger.debug('[ClaudeLocal] Spawn aborted while switching'); + if (!child.pid) { + finalize(); + } + return; + } const message = error instanceof Error ? error.message : String(error); finalize(new Error(`Failed to spawn claude: ${message}. Is Claude installed and on PATH?`)); }); child.on('exit', (code, signal) => { - if (signal === 'SIGTERM' && opts.abort.aborted) { + logger.debug(`[ClaudeLocal] Child exited (code=${code ?? 'null'}, signal=${signal ?? 'null'}, aborted=${opts.abort.aborted}, forced=${forcedTermination}, elapsed=${formatAbortElapsed()})`); + if ((signal === 'SIGTERM' || signal === 'SIGINT' || signal === 'SIGKILL') && opts.abort.aborted) { // Normal termination due to abort signal finalize(); } else if (signal) { @@ -112,6 +202,7 @@ export async function claudeLocal(opts: { }); } finally { process.stdin.resume(); + restoreTerminalState(); } return startFrom ?? null; diff --git a/cli/src/claude/claudeLocalLauncher.ts b/cli/src/claude/claudeLocalLauncher.ts index 678ed924..abaccb0b 100644 --- a/cli/src/claude/claudeLocalLauncher.ts +++ b/cli/src/claude/claudeLocalLauncher.ts @@ -28,7 +28,7 @@ export async function claudeLocalLauncher(session: Session): Promise<'switch' | // Handle abort let exitReason: 'switch' | 'exit' | null = null; const processAbortController = new AbortController(); - let exutFuture = new Future(); + const exitFuture = new Future(); try { async function abort() { @@ -38,7 +38,7 @@ export async function claudeLocalLauncher(session: Session): Promise<'switch' | } // Await full exit - await exutFuture.promise; + await exitFuture.promise; } async function doAbort() { @@ -133,7 +133,7 @@ export async function claudeLocalLauncher(session: Session): Promise<'switch' | } finally { // Resolve future - exutFuture.resolve(undefined); + exitFuture.resolve(undefined); // Set handlers to no-op session.client.rpcHandlerManager.registerHandler('abort', async () => { }); diff --git a/cli/src/claude/claudeRemoteLauncher.ts b/cli/src/claude/claudeRemoteLauncher.ts index a3af3262..eabc5966 100644 --- a/cli/src/claude/claudeRemoteLauncher.ts +++ b/cli/src/claude/claudeRemoteLauncher.ts @@ -15,6 +15,7 @@ import { EnhancedMode } from "./loop"; import { RawJSONLines } from "@/claude/types"; import { OutgoingMessageQueue } from "./utils/OutgoingMessageQueue"; import { getToolName } from "./utils/getToolName"; +import { restoreTerminalState } from "@/ui/terminalState"; interface PermissionsField { date: number; @@ -442,8 +443,9 @@ export async function claudeRemoteLauncher(session: Session): Promise<'switch' | // Reset Terminal process.stdin.off('data', abort); - if (process.stdin.isTTY) { - process.stdin.setRawMode(false); + restoreTerminalState(); + if (hasTTY) { + try { process.stdin.pause(); } catch {} } if (inkInstance) { inkInstance.unmount(); diff --git a/cli/src/claude/runClaude.ts b/cli/src/claude/runClaude.ts index e96218e1..3df284f0 100644 --- a/cli/src/claude/runClaude.ts +++ b/cli/src/claude/runClaude.ts @@ -3,6 +3,7 @@ import { randomUUID } from 'node:crypto'; import { ApiClient } from '@/api/api'; import { logger } from '@/ui/logger'; +import { restoreTerminalState } from '@/ui/terminalState'; import { loop } from '@/claude/loop'; import { AgentState, Metadata } from '@/api/types'; import packageJson from '../../package.json'; @@ -323,6 +324,7 @@ export async function runClaude(options: StartOptions = {}): Promise { // Setup signal handlers for graceful shutdown const cleanup = async () => { logger.debug('[START] Received termination signal, cleaning up...'); + restoreTerminalState(); try { // Update lifecycle state to archived before closing diff --git a/cli/src/ui/ink/useSwitchControls.test.ts b/cli/src/ui/ink/useSwitchControls.test.ts index 5e6a3164..bb5da0ef 100644 --- a/cli/src/ui/ink/useSwitchControls.test.ts +++ b/cli/src/ui/ink/useSwitchControls.test.ts @@ -6,6 +6,8 @@ import { useSwitchControls, type ConfirmationMode, type ActionInProgress } from type Key = { ctrl?: boolean; + name?: string; + sequence?: string; }; type SwitchState = { @@ -194,6 +196,53 @@ describe('useSwitchControls', () => { expect(latestState?.actionInProgress).toBe(null); }); + it('accepts CSI u space sequences', async () => { + const onSwitch = vi.fn(); + await mount({ onSwitch }); + + await triggerInput('\u001b[32u', {}); + expect(latestState?.confirmationMode).toBe('switch'); + }); + + it('accepts CSI u space sequences with modifiers', async () => { + const onSwitch = vi.fn(); + await mount({ onSwitch }); + + await triggerInput('\u001b[32;2u', {}); + expect(latestState?.confirmationMode).toBe('switch'); + }); + + it('ignores CSI u key-release space sequences', async () => { + const onSwitch = vi.fn(); + await mount({ onSwitch }); + + await triggerInput(' ', {}); + expect(latestState?.confirmationMode).toBe('switch'); + + await triggerInput('\u001b[32;2:3u', {}); + expect(latestState?.confirmationMode).toBe('switch'); + expect(onSwitch).not.toHaveBeenCalled(); + }); + + it('accepts space via key name when input is empty', async () => { + const onSwitch = vi.fn(); + await mount({ onSwitch }); + + await triggerInput('', { name: 'space' }); + expect(latestState?.confirmationMode).toBe('switch'); + }); + + it('ignores key-release sequences from key.sequence', async () => { + const onSwitch = vi.fn(); + await mount({ onSwitch }); + + await triggerInput(' ', {}); + expect(latestState?.confirmationMode).toBe('switch'); + + await triggerInput('', { sequence: '\u001b[1:3u' }); + expect(latestState?.confirmationMode).toBe('switch'); + }); + it('does not switch on key-release space sequences', async () => { const onSwitch = vi.fn(); await mount({ onSwitch }); diff --git a/cli/src/ui/ink/useSwitchControls.ts b/cli/src/ui/ink/useSwitchControls.ts index 689609b7..395986a2 100644 --- a/cli/src/ui/ink/useSwitchControls.ts +++ b/cli/src/ui/ink/useSwitchControls.ts @@ -72,8 +72,14 @@ export function useSwitchControls(opts: { return; } - const isKeyRelease = /^\u001b\[[0-9;]*:3u$/.test(input); - const isSpace = Boolean(onSwitch) && !isKeyRelease && input === ' '; + const sequence = typeof key.sequence === 'string' ? key.sequence : input; + const isKeyRelease = typeof sequence === 'string' && /^\u001b\[[0-9;]*:3u$/.test(sequence); + const csiUMatch = typeof sequence === 'string' + ? sequence.match(/^\u001b\[(\d+)(?:;(\d+))?u$/) + : null; + const csiUCodepoint = csiUMatch ? Number(csiUMatch[1]) : null; + const isCsiUSpace = csiUCodepoint === 32; + const isSpace = Boolean(onSwitch) && !isKeyRelease && (input === ' ' || key.name === 'space' || isCsiUSpace); const hasPrintableInput = typeof input === 'string' && input.length > 0; if (isSpace) {