diff --git a/cli/src/commands/command-registry.ts b/cli/src/commands/command-registry.ts index 6738caed9a..f7edfd8700 100644 --- a/cli/src/commands/command-registry.ts +++ b/cli/src/commands/command-registry.ts @@ -311,7 +311,13 @@ const ALL_COMMANDS: CommandDefinition[] = [ name: 'exit', aliases: ['quit', 'q'], handler: () => { - process.kill(process.pid, 'SIGINT') + // Directly exit with cleanup instead of sending SIGINT to our own process. + // process.kill(process.pid, 'SIGINT') would trigger multiple SIGINT + // handlers simultaneously (both renderer-cleanup and use-exit-handler), + // creating a race condition and potentially leaving the terminal buffer + // unflushed. Calling process.exit(0) triggers the 'exit' event handlers + // which run cleanup synchronously before terminating. + process.exit(0) }, }), defineCommandWithArgs({ diff --git a/cli/src/utils/freebuff-exit.ts b/cli/src/utils/freebuff-exit.ts index b444217d02..a87da12f28 100644 --- a/cli/src/utils/freebuff-exit.ts +++ b/cli/src/utils/freebuff-exit.ts @@ -3,11 +3,28 @@ import { endFreebuffSessionBestEffort } from '../hooks/use-freebuff-session' import { flushAnalytics } from './analytics' import { stopActiveRun } from './active-run' import { stopEngagementTracking } from './engagement' +import { TERMINAL_RESET_SEQUENCES } from './terminal-reset-sequences' import { withTimeout } from './terminal-color-detection' /** Cap on exit cleanup so a slow network doesn't block process exit. */ const EXIT_CLEANUP_TIMEOUT_MS = 1_000 +/** + * Ensure any buffered terminal output is written to the terminal before the + * process exits. Without this flush, process.exit() can terminate without + * sending pending terminal escape sequences, leaving garbled output and + * potentially causing ASCII/UTF-8 decoding errors in the terminal. + */ +function flushTerminalOutput(): void { + try { + if (process.stdout.isTTY) { + process.stdout.write(TERMINAL_RESET_SEQUENCES) + } + } catch { + // stdout may be closed + } +} + /** * Flush analytics + release the freebuff seat (best-effort), then exit 0. * Shared by every freebuff-specific screen's Ctrl+C / X handler so they all @@ -23,5 +40,9 @@ export async function exitFreebuffCleanly(): Promise { EXIT_CLEANUP_TIMEOUT_MS, undefined, ) + // Flush terminal output before exiting to prevent garbled terminal state. + // This writes terminal reset sequences and ensures they reach the terminal + // before the process terminates. + flushTerminalOutput() process.exit(0) } diff --git a/cli/src/utils/renderer-cleanup.ts b/cli/src/utils/renderer-cleanup.ts index 028e273d30..65293e8ff8 100644 --- a/cli/src/utils/renderer-cleanup.ts +++ b/cli/src/utils/renderer-cleanup.ts @@ -18,6 +18,12 @@ let terminalStateReset = false * * This is especially important on Windows where signals like SIGTERM and SIGHUP * don't work, so we rely on the 'exit' event which is guaranteed to run. + * + * After writing the reset sequences, we attempt to flush stdout to ensure the + * data reaches the terminal before the process exits. Without this flush, a + * sudden process.exit() can leave terminal escape sequences buffered and never + * sent, causing garbled output and ASCII/UTF-8 decoding errors on the next + * terminal prompt. */ function resetTerminalState(): void { if (terminalStateReset) return @@ -37,6 +43,10 @@ function resetTerminalState(): void { // before the process exits, ensuring the terminal is reset if (process.stdout.isTTY) { process.stdout.write(TERMINAL_RESET_SEQUENCES) + // NOTE: do NOT call destroy() here — that discards buffered data. + // TTY writes are synchronous (write() syscall goes directly to the + // PTY), so the data reaches the kernel buffer before the call returns. + // process.exit() then terminates cleanly and the kernel flushes fd 1. } } catch { // Ignore errors - stdout may already be closed @@ -96,6 +106,14 @@ export function installProcessCleanupHandlers(cliRenderer: CliRenderer): void { const cleanupAndExit = (exitCode: number) => { cleanup() + // Ensure stdout and stderr are drained before exit. Without this, pending + // writes (e.g. terminal reset sequences from cleanup()) may be buffered + // and lost, leaving the terminal in a garbled state. + try { + process.stdout._handle?.setBlocking?.(true) + } catch { + // _handle may not exist in Bun or on some platforms + } process.exit(exitCode) } @@ -121,6 +139,11 @@ export function installProcessCleanupHandlers(cliRenderer: CliRenderer): void { // exit - Last chance to run synchronous cleanup code process.on('exit', () => { + // Guard: prevent double-cleanup if this is called from cleanupAndExit + // (which calls cleanup() before process.exit(), which triggers this + // 'exit' event handler and calls cleanup() again). + if (!handlersInstalled) return + handlersInstalled = false cleanup() }) diff --git a/sdk/src/tools/run-terminal-command.ts b/sdk/src/tools/run-terminal-command.ts index 30dbd86ea9..ce3073bd4c 100644 --- a/sdk/src/tools/run-terminal-command.ts +++ b/sdk/src/tools/run-terminal-command.ts @@ -90,6 +90,14 @@ export class BoundedOutputBuffer { } function killProcessGroup(child: ChildProcess, signal: NodeJS.Signals) { + // SAFETY: Never kill our own process. If a child somehow has the same pid + // as this process (e.g. pid reuse race, or the child ran a command that + // reports the parent's pid), skip it to prevent self-kill which would + // leave the terminal buffer unflushed and cause ASCII/UTF-8 decoding errors. + if (child.pid === process.pid) { + return + } + if (os.platform() === 'win32' && child.pid) { // Node's child.kill() only terminates the direct process on Windows. Since // the direct process is Git Bash, killing it first can orphan Bun/Node @@ -168,6 +176,10 @@ function installExitSweep() { exitSweepInstalled = true process.on('exit', () => { for (const child of liveChildren) { + // SAFETY: Never kill our own process in the exit sweep. liveChildren + // should never contain process.pid, but guard defensively to prevent + // the self-kill scenario that would leave the terminal buffer unflushed. + if (child.pid === process.pid) continue killProcessGroup(child, 'SIGKILL') } })