From 5b820c5ce759158d274c22939eccc4839cb49c49 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Wed, 16 Jul 2025 08:59:26 -0700 Subject: [PATCH 1/2] feat: ctrl-d only exits when there is no user input (#1589) While this does make it so that `ctrl-d` will not exit Codex when the composer is not empty, `ctrl-d` will still exit Codex if it is in the "working" state. Fixes https://github.com/openai/codex/issues/1443. --- codex-rs/tui/src/app.rs | 16 +++++++++++++++- codex-rs/tui/src/bottom_pane/chat_composer.rs | 5 +++++ .../tui/src/bottom_pane/chat_composer_history.rs | 4 ++-- codex-rs/tui/src/bottom_pane/mod.rs | 4 ++++ codex-rs/tui/src/chatwidget.rs | 4 ++++ 5 files changed, 30 insertions(+), 3 deletions(-) diff --git a/codex-rs/tui/src/app.rs b/codex-rs/tui/src/app.rs index e1dde8332d..33297ad372 100644 --- a/codex-rs/tui/src/app.rs +++ b/codex-rs/tui/src/app.rs @@ -199,7 +199,21 @@ impl<'a> App<'a> { modifiers: crossterm::event::KeyModifiers::CONTROL, .. } => { - self.app_event_tx.send(AppEvent::ExitRequest); + match &mut self.app_state { + AppState::Chat { widget } => { + if widget.composer_is_empty() { + self.app_event_tx.send(AppEvent::ExitRequest); + } else { + // Treat Ctrl+D as a normal key event when the composer + // is not empty so that it doesn't quit the application + // prematurely. + self.dispatch_key_event(key_event); + } + } + AppState::Login { .. } | AppState::GitWarning { .. } => { + self.app_event_tx.send(AppEvent::ExitRequest); + } + } } _ => { self.dispatch_key_event(key_event); diff --git a/codex-rs/tui/src/bottom_pane/chat_composer.rs b/codex-rs/tui/src/bottom_pane/chat_composer.rs index e89187d165..b49bce4046 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer.rs @@ -76,6 +76,11 @@ impl ChatComposer<'_> { this } + /// Returns true if the composer currently contains no user input. + pub(crate) fn is_empty(&self) -> bool { + self.textarea.is_empty() + } + /// Update the cached *context-left* percentage and refresh the placeholder /// text. The UI relies on the placeholder to convey the remaining /// context when the composer is empty. diff --git a/codex-rs/tui/src/bottom_pane/chat_composer_history.rs b/codex-rs/tui/src/bottom_pane/chat_composer_history.rs index fc85c28262..5715c99492 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer_history.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer_history.rs @@ -72,8 +72,7 @@ impl ChatComposerHistory { return false; } - let lines = textarea.lines(); - if lines.len() == 1 && lines[0].is_empty() { + if textarea.is_empty() { return true; } @@ -85,6 +84,7 @@ impl ChatComposerHistory { return false; } + let lines = textarea.lines(); matches!(&self.last_history_text, Some(prev) if prev == &lines.join("\n")) } diff --git a/codex-rs/tui/src/bottom_pane/mod.rs b/codex-rs/tui/src/bottom_pane/mod.rs index 350492b3e9..e4ea1d3823 100644 --- a/codex-rs/tui/src/bottom_pane/mod.rs +++ b/codex-rs/tui/src/bottom_pane/mod.rs @@ -162,6 +162,10 @@ impl BottomPane<'_> { } } + pub(crate) fn composer_is_empty(&self) -> bool { + self.composer.is_empty() + } + pub(crate) fn is_task_running(&self) -> bool { self.is_task_running } diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 865e339763..51fdfc3e8a 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -432,6 +432,10 @@ impl ChatWidget<'_> { } } + pub(crate) fn composer_is_empty(&self) -> bool { + self.bottom_pane.composer_is_empty() + } + /// Forward an `Op` directly to codex. pub(crate) fn submit_op(&self, op: Op) { if let Err(e) = self.codex_op_tx.send(op) { From 40e9f725ae0c3c48c1d765eb0653afddaeaa39ea Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Wed, 16 Jul 2025 09:33:51 -0700 Subject: [PATCH 2/2] fix: update bin/codex.js so it listens for exit on the child process --- codex-cli/bin/codex.js | 76 +++++++++++++++++++++++++++++++++++------- 1 file changed, 64 insertions(+), 12 deletions(-) diff --git a/codex-cli/bin/codex.js b/codex-cli/bin/codex.js index 54b99078e4..f291d8c49f 100755 --- a/codex-cli/bin/codex.js +++ b/codex-cli/bin/codex.js @@ -15,7 +15,7 @@ * current platform / architecture, an error is thrown. */ -import { spawnSync } from "child_process"; +// Only imported dynamically when needed – see below. import fs from "fs"; import path from "path"; import { fileURLToPath, pathToFileURL } from "url"; @@ -74,22 +74,74 @@ if (wantsNative) { } const binaryPath = path.join(__dirname, "..", "bin", `codex-${targetTriple}`); - const result = spawnSync(binaryPath, process.argv.slice(2), { + + /* + * Use an asynchronous spawn instead of spawnSync so that Node is able to + * respond to signals (e.g. Ctrl-C / SIGINT) while the native binary is + * executing. This allows us to forward those signals to the child process + * and guarantees that when either the child terminates or the parent + * receives a fatal signal, both processes exit in a predictable manner. + */ + + const { spawn } = await import("child_process"); + + const child = spawn(binaryPath, process.argv.slice(2), { stdio: "inherit", }); - const exitCode = typeof result.status === "number" ? result.status : 1; - process.exit(exitCode); -} + child.on("error", (err) => { + // Typically triggered when the binary is missing or not executable. + // Re-throwing here will terminate the parent with a non-zero exit code + // while still printing a helpful stack trace. + console.error(err); + process.exit(1); + }); -// Fallback: execute the original JavaScript CLI. + // Forward common termination signals to the child so that it shuts down + // gracefully. In the handler we temporarily disable the default behaviour of + // exiting immediately; once the child has been signalled we simply wait for + // its exit event which will in turn terminate the parent (see below). + const forwardSignal = (signal) => { + if (child.killed) { + return; + } + try { + child.kill(signal); + } catch { + /* ignore */ + } + }; -// Resolve the path to the compiled CLI bundle -const cliPath = path.resolve(__dirname, "../dist/cli.js"); -const cliUrl = pathToFileURL(cliPath).href; + ["SIGINT", "SIGTERM", "SIGHUP"].forEach((sig) => { + process.on(sig, () => forwardSignal(sig)); + }); -// Load and execute the CLI -(async () => { + // When the child exits, mirror its termination reason in the parent so that + // shell scripts and other tooling observe the correct exit status. + child.on("exit", (code, signal) => { + if (signal) { + // Re-emit the same signal so that the parent terminates with the + // expected semantics (this also sets the correct exit code of 128 + n). + process.kill(process.pid, signal); + } else { + process.exit(code ?? 1); + } + }); + + // There is nothing more for the parent to do here – wait until the child + // terminates or a signal is received. We deliberately do *not* continue on + // to the JavaScript CLI fallback below, so we simply return from the current + // module evaluation by awaiting the child's "exit" event. + + await new Promise(() => {}); +} else { + // Fallback: execute the original JavaScript CLI. + + // Resolve the path to the compiled CLI bundle + const cliPath = path.resolve(__dirname, "../dist/cli.js"); + const cliUrl = pathToFileURL(cliPath).href; + + // Load and execute the CLI try { await import(cliUrl); } catch (err) { @@ -97,4 +149,4 @@ const cliUrl = pathToFileURL(cliPath).href; console.error(err); process.exit(1); } -})(); +}