diff --git a/cli/src/claude/claudeLocal.ts b/cli/src/claude/claudeLocal.ts index 5e3e27cf..f6f4fbeb 100644 --- a/cli/src/claude/claudeLocal.ts +++ b/cli/src/claude/claudeLocal.ts @@ -1,12 +1,11 @@ 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 { appendMcpConfigArg } from "./utils/mcpConfig"; import { systemPrompt } from "./utils/systemPrompt"; import { withBunRuntimeEnv } from "@/utils/bunRuntime"; -import { spawnWithAbort } from "@/utils/spawnWithAbort"; +import { spawnWithTerminalGuard } from "@/utils/spawnWithTerminalGuard"; import { getHapiBlobsDir } from "@/constants/uploadPaths"; import { stripNewlinesForWindowsShellArg } from "@/utils/shellEscape"; import { getDefaultClaudeCodePath } from "./sdk/utils"; @@ -91,8 +90,7 @@ export async function claudeLocal(opts: { // Spawn the process try { - process.stdin.pause(); - await spawnWithAbort({ + await spawnWithTerminalGuard({ command: claudeCommand, args, cwd: opts.path, @@ -107,8 +105,6 @@ export async function claudeLocal(opts: { }); } finally { cleanupMcpConfig?.(); - process.stdin.resume(); - restoreTerminalState(); } return startFrom ?? null; diff --git a/cli/src/codex/codexLocal.ts b/cli/src/codex/codexLocal.ts index c4940b89..b2399744 100644 --- a/cli/src/codex/codexLocal.ts +++ b/cli/src/codex/codexLocal.ts @@ -1,6 +1,5 @@ import { logger } from '@/ui/logger'; -import { restoreTerminalState } from '@/ui/terminalState'; -import { spawnWithAbort } from '@/utils/spawnWithAbort'; +import { spawnWithTerminalGuard } from '@/utils/spawnWithTerminalGuard'; import { buildMcpServerConfigArgs, buildDeveloperInstructionsArg } from './utils/codexMcpConfig'; import { codexSystemPrompt } from './utils/systemPrompt'; @@ -68,23 +67,17 @@ export async function codexLocal(opts: { return; } - process.stdin.pause(); - try { - await spawnWithAbort({ - command: 'codex', - args, - cwd: opts.path, - env: process.env, - signal: opts.abort, - logLabel: 'CodexLocal', - spawnName: 'codex', - installHint: 'Codex CLI', - includeCause: true, - logExit: true, - shell: process.platform === 'win32' - }); - } finally { - process.stdin.resume(); - restoreTerminalState(); - } + await spawnWithTerminalGuard({ + command: 'codex', + args, + cwd: opts.path, + env: process.env, + signal: opts.abort, + logLabel: 'CodexLocal', + spawnName: 'codex', + installHint: 'Codex CLI', + includeCause: true, + logExit: true, + shell: process.platform === 'win32' + }); } diff --git a/cli/src/cursor/cursorLocal.ts b/cli/src/cursor/cursorLocal.ts index 129d0b0c..a979f68e 100644 --- a/cli/src/cursor/cursorLocal.ts +++ b/cli/src/cursor/cursorLocal.ts @@ -1,6 +1,5 @@ import { logger } from '@/ui/logger'; -import { restoreTerminalState } from '@/ui/terminalState'; -import { spawnWithAbort } from '@/utils/spawnWithAbort'; +import { spawnWithTerminalGuard } from '@/utils/spawnWithTerminalGuard'; /** * Filter out 'resume' subcommand which is managed internally by hapi. @@ -61,23 +60,17 @@ export async function cursorLocal(opts: { return; } - process.stdin.pause(); - try { - await spawnWithAbort({ - command: 'agent', - args, - cwd: opts.path, - env: process.env, - signal: opts.abort, - logLabel: 'CursorLocal', - spawnName: 'agent', - installHint: 'Cursor Agent CLI (curl https://cursor.com/install -fsS | bash)', - includeCause: true, - logExit: true, - shell: process.platform === 'win32' - }); - } finally { - process.stdin.resume(); - restoreTerminalState(); - } + await spawnWithTerminalGuard({ + command: 'agent', + args, + cwd: opts.path, + env: process.env, + signal: opts.abort, + logLabel: 'CursorLocal', + spawnName: 'agent', + installHint: 'Cursor Agent CLI (curl https://cursor.com/install -fsS | bash)', + includeCause: true, + logExit: true, + shell: process.platform === 'win32' + }); } diff --git a/cli/src/cursor/cursorRemoteLauncher.ts b/cli/src/cursor/cursorRemoteLauncher.ts index 6346ff68..b957edc9 100644 --- a/cli/src/cursor/cursorRemoteLauncher.ts +++ b/cli/src/cursor/cursorRemoteLauncher.ts @@ -2,6 +2,7 @@ import React from 'react'; import { spawn } from 'node:child_process'; import { createInterface } from 'node:readline'; import { logger } from '@/ui/logger'; +import { killProcessByChildProcess } from '@/utils/process'; import { convertAgentMessage } from '@/agent/messageConverter'; import { OpencodeDisplay } from '@/ui/ink/OpencodeDisplay'; import { @@ -177,11 +178,7 @@ class CursorRemoteLauncher extends RemoteLauncherBase { }); const abortHandler = () => { - try { - child.kill('SIGTERM'); - } catch { - // ignore - } + killProcessByChildProcess(child, false).catch(() => {}); resolve(null); }; this.abortController.signal.addEventListener('abort', abortHandler); diff --git a/cli/src/gemini/geminiLocal.ts b/cli/src/gemini/geminiLocal.ts index 0cd86ffa..551145df 100644 --- a/cli/src/gemini/geminiLocal.ts +++ b/cli/src/gemini/geminiLocal.ts @@ -1,6 +1,5 @@ import { logger } from '@/ui/logger'; -import { restoreTerminalState } from '@/ui/terminalState'; -import { spawnWithAbort } from '@/utils/spawnWithAbort'; +import { spawnWithTerminalGuard } from '@/utils/spawnWithTerminalGuard'; export async function geminiLocal(opts: { path: string; @@ -36,23 +35,17 @@ export async function geminiLocal(opts: { logger.debug(`[GeminiLocal] Spawning gemini with args: ${JSON.stringify(args)}`); - process.stdin.pause(); - try { - await spawnWithAbort({ - command: 'gemini', - args, - cwd: opts.path, - env, - signal: opts.abort, - shell: process.platform === 'win32', - logLabel: 'GeminiLocal', - spawnName: 'gemini', - installHint: 'Gemini CLI', - includeCause: true, - logExit: true - }); - } finally { - process.stdin.resume(); - restoreTerminalState(); - } + await spawnWithTerminalGuard({ + command: 'gemini', + args, + cwd: opts.path, + env, + signal: opts.abort, + shell: process.platform === 'win32', + logLabel: 'GeminiLocal', + spawnName: 'gemini', + installHint: 'Gemini CLI', + includeCause: true, + logExit: true + }); } diff --git a/cli/src/opencode/opencodeLocal.ts b/cli/src/opencode/opencodeLocal.ts index 7dc9d03c..3b2649a1 100644 --- a/cli/src/opencode/opencodeLocal.ts +++ b/cli/src/opencode/opencodeLocal.ts @@ -1,6 +1,5 @@ import { logger } from '@/ui/logger'; -import { restoreTerminalState } from '@/ui/terminalState'; -import { spawnWithAbort } from '@/utils/spawnWithAbort'; +import { spawnWithTerminalGuard } from '@/utils/spawnWithTerminalGuard'; export async function opencodeLocal(opts: { path: string; @@ -15,23 +14,17 @@ export async function opencodeLocal(opts: { logger.debug(`[OpencodeLocal] Spawning opencode with args: ${JSON.stringify(args)}`); - process.stdin.pause(); - try { - await spawnWithAbort({ - command: 'opencode', - args, - cwd: opts.path, - env: opts.env, - signal: opts.abort, - shell: process.platform === 'win32', - logLabel: 'OpencodeLocal', - spawnName: 'opencode', - installHint: 'OpenCode CLI', - includeCause: true, - logExit: true - }); - } finally { - process.stdin.resume(); - restoreTerminalState(); - } + await spawnWithTerminalGuard({ + command: 'opencode', + args, + cwd: opts.path, + env: opts.env, + signal: opts.abort, + shell: process.platform === 'win32', + logLabel: 'OpencodeLocal', + spawnName: 'opencode', + installHint: 'OpenCode CLI', + includeCause: true, + logExit: true + }); } diff --git a/cli/src/utils/spawnWithAbort.ts b/cli/src/utils/spawnWithAbort.ts index 9ecc145f..00b54cc0 100644 --- a/cli/src/utils/spawnWithAbort.ts +++ b/cli/src/utils/spawnWithAbort.ts @@ -148,3 +148,5 @@ export async function spawnWithAbort(options: SpawnWithAbortOptions): Promise ({ + restoreTerminalState: vi.fn(), +})); + +vi.mock('@/utils/spawnWithAbort', () => ({ + spawnWithAbort: vi.fn(), +})); + +import { spawnWithTerminalGuard } from '@/utils/spawnWithTerminalGuard'; +import { spawnWithAbort } from '@/utils/spawnWithAbort'; +import { restoreTerminalState } from '@/ui/terminalState'; + +const mockSpawnWithAbort = vi.mocked(spawnWithAbort); +const mockRestoreTerminalState = vi.mocked(restoreTerminalState); + +const dummyOptions = { + command: 'test-agent', + args: [], + cwd: '/tmp', + env: process.env, + signal: new AbortController().signal, + logLabel: 'Test', + spawnName: 'test', + installHint: 'Test CLI', +}; + +describe('spawnWithTerminalGuard', () => { + let stdinPauseSpy: ReturnType; + let stdinResumeSpy: ReturnType; + + beforeEach(() => { + vi.clearAllMocks(); + stdinPauseSpy = vi.spyOn(process.stdin, 'pause').mockImplementation(() => process.stdin); + stdinResumeSpy = vi.spyOn(process.stdin, 'resume').mockImplementation(() => process.stdin); + }); + + afterEach(() => { + stdinPauseSpy.mockRestore(); + stdinResumeSpy.mockRestore(); + }); + + it('pauses stdin before spawn and resumes after success', async () => { + mockSpawnWithAbort.mockResolvedValue(); + + await spawnWithTerminalGuard(dummyOptions); + + expect(stdinPauseSpy).toHaveBeenCalledOnce(); + expect(stdinResumeSpy).toHaveBeenCalledOnce(); + expect(mockRestoreTerminalState).toHaveBeenCalledOnce(); + }); + + it('passes options through to spawnWithAbort unchanged', async () => { + mockSpawnWithAbort.mockResolvedValue(); + + await spawnWithTerminalGuard(dummyOptions); + + expect(mockSpawnWithAbort).toHaveBeenCalledWith(dummyOptions); + }); + + it('resumes stdin and restores terminal state even when spawn rejects', async () => { + mockSpawnWithAbort.mockRejectedValue(new Error('spawn failed')); + + await expect(spawnWithTerminalGuard(dummyOptions)).rejects.toThrow('spawn failed'); + + expect(stdinResumeSpy).toHaveBeenCalledOnce(); + expect(mockRestoreTerminalState).toHaveBeenCalledOnce(); + }); + + it('propagates the original error from spawnWithAbort', async () => { + const error = new Error('process exited with code 1'); + mockSpawnWithAbort.mockRejectedValue(error); + + await expect(spawnWithTerminalGuard(dummyOptions)).rejects.toThrow(error); + }); + + it('calls pause before spawn, and resume after spawn', async () => { + mockSpawnWithAbort.mockResolvedValue(); + + await spawnWithTerminalGuard(dummyOptions); + + expect(stdinPauseSpy.mock.invocationCallOrder[0]) + .toBeLessThan(mockSpawnWithAbort.mock.invocationCallOrder[0]); + expect(mockSpawnWithAbort.mock.invocationCallOrder[0]) + .toBeLessThan(stdinResumeSpy.mock.invocationCallOrder[0]); + }); +}); diff --git a/cli/src/utils/spawnWithTerminalGuard.ts b/cli/src/utils/spawnWithTerminalGuard.ts new file mode 100644 index 00000000..b7361c70 --- /dev/null +++ b/cli/src/utils/spawnWithTerminalGuard.ts @@ -0,0 +1,16 @@ +import { restoreTerminalState } from '@/ui/terminalState'; +import { spawnWithAbort, type SpawnWithAbortOptions } from '@/utils/spawnWithAbort'; + +/** + * Guards the terminal around a spawnWithAbort call: pauses stdin before spawn, + * then resumes stdin and restores terminal escape state in finally. + */ +export async function spawnWithTerminalGuard(options: SpawnWithAbortOptions): Promise { + process.stdin.pause(); + try { + await spawnWithAbort(options); + } finally { + process.stdin.resume(); + restoreTerminalState(); + } +}