mirror of
https://github.com/openai/codex.git
synced 2026-11-05 20:10:33 +00:00
Add OSC 777 notifications and improve approval notification text (#59256)
## What changed - Support `tui.notification_method = "osc777"` with a `Codex` title and tmux passthrough. - Replace control characters in OSC notification bodies and escape semicolon delimiters for OSC 777. Limit Ghostty bodies to 255 bytes, preserving UTF-8 boundaries and adding an ellipsis when truncated. - Remove the 30-grapheme command limit from approval notifications, strip shell wrappers, and describe terminal input and network access requests explicitly. ## Testing Add coverage for notification method selection, OSC 777 output with and without tmux, control character handling, Ghostty truncation, and approval notification text. GitOrigin-RevId: e15fa7fdb007c38a148217f6aa0bbcc5e7371f9e
This commit is contained in:
@@ -744,6 +744,7 @@ pub enum NotificationMethod {
|
||||
#[default]
|
||||
Auto,
|
||||
Osc9,
|
||||
Osc777,
|
||||
Bel,
|
||||
}
|
||||
|
||||
@@ -752,6 +753,7 @@ impl fmt::Display for NotificationMethod {
|
||||
match self {
|
||||
NotificationMethod::Auto => write!(f, "auto"),
|
||||
NotificationMethod::Osc9 => write!(f, "osc9"),
|
||||
NotificationMethod::Osc777 => write!(f, "osc777"),
|
||||
NotificationMethod::Bel => write!(f, "bel"),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -3744,6 +3744,7 @@
|
||||
"enum": [
|
||||
"auto",
|
||||
"osc9",
|
||||
"osc777",
|
||||
"bel"
|
||||
],
|
||||
"type": "string"
|
||||
|
||||
@@ -26,7 +26,7 @@ impl ChatWidget {
|
||||
#[derive(Debug)]
|
||||
pub(crate) enum Notification {
|
||||
AgentTurnComplete { response: String },
|
||||
ExecApprovalRequested { command: String },
|
||||
ExecApprovalRequested { subject: String },
|
||||
EditApprovalRequested { cwd: PathBuf, changes: Vec<PathBuf> },
|
||||
ElicitationRequested { server_name: String },
|
||||
PlanModePrompt { title: String },
|
||||
@@ -40,11 +40,8 @@ impl Notification {
|
||||
Notification::agent_turn_preview(response)
|
||||
.unwrap_or_else(|| "Agent turn complete".to_string())
|
||||
}
|
||||
Notification::ExecApprovalRequested { command } => {
|
||||
format!(
|
||||
"Approval requested: {}",
|
||||
truncate_text(command, /*max_graphemes*/ 30)
|
||||
)
|
||||
Notification::ExecApprovalRequested { subject } => {
|
||||
format!("Approval requested: {subject}")
|
||||
}
|
||||
Notification::EditApprovalRequested { cwd, changes } => {
|
||||
format!(
|
||||
|
||||
@@ -49,8 +49,9 @@ async fn exec_approval_emits_proposed_command_and_decision_history() {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn app_server_exec_approval_request_splits_shell_wrapped_command() {
|
||||
#[tokio::test]
|
||||
async fn app_server_exec_approval_request_splits_shell_wrapped_command() {
|
||||
let (mut chat, _rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
let script = r#"python3 -c 'print("Hello, world!")'"#;
|
||||
let request = exec_approval_request_from_params(
|
||||
AppServerCommandExecutionRequestApprovalParams {
|
||||
@@ -85,6 +86,12 @@ fn app_server_exec_approval_request_splits_shell_wrapped_command() {
|
||||
script.to_string(),
|
||||
]
|
||||
);
|
||||
|
||||
handle_exec_approval_request(&mut chat, "approval-1", request);
|
||||
assert_snapshot!(
|
||||
chat.pending_notification.as_ref().expect("approval notification").display(),
|
||||
@r#"Approval requested: python3 -c 'print("Hello, world!")'"#
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
@@ -116,6 +123,10 @@ async fn app_server_write_stdin_approval_renders_terminal_input() {
|
||||
);
|
||||
|
||||
handle_exec_approval_request(&mut chat, "approval-1", event);
|
||||
assert_snapshot!(
|
||||
chat.pending_notification.as_ref().expect("approval notification").display(),
|
||||
@"Approval requested: send terminal input"
|
||||
);
|
||||
|
||||
let area = Rect::new(0, 0, 80, chat.desired_height(/*width*/ 80));
|
||||
let mut buf = ratatui::buffer::Buffer::empty(area);
|
||||
@@ -221,6 +232,10 @@ async fn network_exec_approval_history_describes_session_host_allowance() {
|
||||
);
|
||||
|
||||
handle_exec_approval_request(&mut chat, "sub-network", request);
|
||||
assert_snapshot!(
|
||||
chat.pending_notification.as_ref().expect("approval notification").display(),
|
||||
@"Approval requested: network access to example.com"
|
||||
);
|
||||
chat.handle_key_event(KeyEvent::new(KeyCode::Char('a'), KeyModifiers::NONE));
|
||||
|
||||
let decision = drain_insert_history(&mut rx)
|
||||
|
||||
@@ -4,6 +4,7 @@
|
||||
//! that block on user decisions.
|
||||
|
||||
use super::*;
|
||||
use codex_app_server_protocol::CommandExecutionApprovalKind;
|
||||
|
||||
impl ChatWidget {
|
||||
pub(super) fn on_exec_approval_request(&mut self, _id: String, ev: ExecApprovalRequestEvent) {
|
||||
@@ -282,9 +283,14 @@ impl ChatWidget {
|
||||
|
||||
pub(crate) fn handle_exec_approval_now(&mut self, ev: ExecApprovalRequestEvent) {
|
||||
self.flush_answer_stream_with_separator();
|
||||
let command = shlex::try_join(ev.command.iter().map(String::as_str))
|
||||
.unwrap_or_else(|_| ev.command.join(" "));
|
||||
self.notify(Notification::ExecApprovalRequested { command });
|
||||
let subject = if ev.kind == CommandExecutionApprovalKind::WriteStdin {
|
||||
"send terminal input".to_string()
|
||||
} else if let Some(network) = &ev.network_approval_context {
|
||||
format!("network access to {}", network.host)
|
||||
} else {
|
||||
strip_bash_lc_and_escape(&ev.command)
|
||||
};
|
||||
self.notify(Notification::ExecApprovalRequested { subject });
|
||||
|
||||
let available_decisions = ev.effective_available_decisions();
|
||||
let request = ApprovalRequest::Exec(ExecApprovalRequest {
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
mod bel;
|
||||
mod osc9;
|
||||
mod osc;
|
||||
|
||||
use std::io;
|
||||
|
||||
@@ -8,11 +8,12 @@ use codex_config::types::NotificationMethod;
|
||||
use codex_terminal_detection::TerminalInfo;
|
||||
use codex_terminal_detection::TerminalName;
|
||||
use codex_terminal_detection::terminal_info;
|
||||
use osc9::Osc9Backend;
|
||||
use osc::OscBackend;
|
||||
use osc::OscProtocol;
|
||||
|
||||
#[derive(Debug)]
|
||||
pub enum DesktopNotificationBackend {
|
||||
Osc9(Osc9Backend),
|
||||
Osc(OscBackend),
|
||||
Bel(BelBackend),
|
||||
}
|
||||
|
||||
@@ -21,26 +22,30 @@ impl DesktopNotificationBackend {
|
||||
match method {
|
||||
NotificationMethod::Auto => {
|
||||
if supports_osc9(&terminal_info()) {
|
||||
Self::Osc9(Osc9Backend::new())
|
||||
Self::Osc(OscBackend::new(OscProtocol::Osc9))
|
||||
} else {
|
||||
Self::Bel(BelBackend)
|
||||
}
|
||||
}
|
||||
NotificationMethod::Osc9 => Self::Osc9(Osc9Backend::new()),
|
||||
NotificationMethod::Osc9 => Self::Osc(OscBackend::new(OscProtocol::Osc9)),
|
||||
NotificationMethod::Osc777 => Self::Osc(OscBackend::new(OscProtocol::Osc777)),
|
||||
NotificationMethod::Bel => Self::Bel(BelBackend),
|
||||
}
|
||||
}
|
||||
|
||||
pub fn method(&self) -> NotificationMethod {
|
||||
match self {
|
||||
DesktopNotificationBackend::Osc9(_) => NotificationMethod::Osc9,
|
||||
DesktopNotificationBackend::Osc(backend) => match backend.protocol {
|
||||
OscProtocol::Osc9 => NotificationMethod::Osc9,
|
||||
OscProtocol::Osc777 => NotificationMethod::Osc777,
|
||||
},
|
||||
DesktopNotificationBackend::Bel(_) => NotificationMethod::Bel,
|
||||
}
|
||||
}
|
||||
|
||||
pub fn notify(&mut self, message: &str) -> io::Result<()> {
|
||||
match self {
|
||||
DesktopNotificationBackend::Osc9(backend) => backend.notify(message),
|
||||
DesktopNotificationBackend::Osc(backend) => backend.notify(message),
|
||||
DesktopNotificationBackend::Bel(backend) => backend.notify(message),
|
||||
}
|
||||
}
|
||||
@@ -82,19 +87,14 @@ mod tests {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn selects_osc9_method() {
|
||||
assert!(matches!(
|
||||
detect_backend(NotificationMethod::Osc9),
|
||||
super::DesktopNotificationBackend::Osc9(_)
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn selects_bel_method() {
|
||||
assert!(matches!(
|
||||
detect_backend(NotificationMethod::Bel),
|
||||
super::DesktopNotificationBackend::Bel(_)
|
||||
));
|
||||
fn selects_explicit_notification_method() {
|
||||
for method in [
|
||||
NotificationMethod::Osc9,
|
||||
NotificationMethod::Osc777,
|
||||
NotificationMethod::Bel,
|
||||
] {
|
||||
assert_eq!(detect_backend(method).method(), method);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -0,0 +1,210 @@
|
||||
//! Writes terminal desktop notifications, wrapping the OSC sequence for tmux when needed.
|
||||
|
||||
use std::fmt;
|
||||
use std::io;
|
||||
use std::io::stdout;
|
||||
|
||||
use codex_terminal_detection::Multiplexer;
|
||||
use codex_terminal_detection::TerminalName;
|
||||
use codex_terminal_detection::terminal_info;
|
||||
use crossterm::Command;
|
||||
use ratatui::crossterm::execute;
|
||||
|
||||
// Ghostty copies the parsed notification body into a 255-byte buffer.
|
||||
const GHOSTTY_BODY_BYTES: usize = 255;
|
||||
|
||||
#[derive(Debug, Clone, Copy)]
|
||||
pub(super) enum OscProtocol {
|
||||
Osc9,
|
||||
Osc777,
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
pub struct OscBackend {
|
||||
pub(super) protocol: OscProtocol,
|
||||
terminal_name: TerminalName,
|
||||
dcs_passthrough: bool,
|
||||
}
|
||||
|
||||
impl OscBackend {
|
||||
pub(super) fn new(protocol: OscProtocol) -> Self {
|
||||
let terminal = terminal_info();
|
||||
Self {
|
||||
protocol,
|
||||
terminal_name: terminal.name,
|
||||
dcs_passthrough: matches!(terminal.multiplexer, Some(Multiplexer::Tmux { .. })),
|
||||
}
|
||||
}
|
||||
|
||||
pub fn notify(&mut self, message: &str) -> io::Result<()> {
|
||||
execute!(
|
||||
stdout(),
|
||||
PostNotification {
|
||||
protocol: self.protocol,
|
||||
message,
|
||||
terminal_name: self.terminal_name,
|
||||
dcs_passthrough: self.dcs_passthrough,
|
||||
}
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
struct PostNotification<'a> {
|
||||
protocol: OscProtocol,
|
||||
message: &'a str,
|
||||
terminal_name: TerminalName,
|
||||
dcs_passthrough: bool,
|
||||
}
|
||||
|
||||
impl Command for PostNotification<'_> {
|
||||
fn write_ansi(&self, f: &mut impl fmt::Write) -> fmt::Result {
|
||||
let mut body: String = self
|
||||
.message
|
||||
.chars()
|
||||
.map(|c| match c {
|
||||
// ASCII semicolons delimit OSC 777 fields.
|
||||
';' if matches!(self.protocol, OscProtocol::Osc777) => ';',
|
||||
c if c.is_control() => ' ',
|
||||
c => c,
|
||||
})
|
||||
.collect();
|
||||
if self.terminal_name == TerminalName::Ghostty && body.len() > GHOSTTY_BODY_BYTES {
|
||||
body.truncate(body.floor_char_boundary(GHOSTTY_BODY_BYTES - '…'.len_utf8()));
|
||||
body.push('…');
|
||||
}
|
||||
let sequence = match self.protocol {
|
||||
OscProtocol::Osc9 => format!("\x1b]9;{body}\x07"),
|
||||
OscProtocol::Osc777 => format!("\x1b]777;notify;Codex;{body}\x1b\\"),
|
||||
};
|
||||
if self.dcs_passthrough {
|
||||
let escaped = sequence.replace('\x1b', "\x1b\x1b");
|
||||
write!(f, "\x1bPtmux;{escaped}\x1b\\")
|
||||
} else {
|
||||
f.write_str(&sequence)
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(windows)]
|
||||
fn execute_winapi(&self) -> io::Result<()> {
|
||||
Err(std::io::Error::other(
|
||||
"tried to execute PostNotification using WinAPI; use ANSI instead",
|
||||
))
|
||||
}
|
||||
|
||||
#[cfg(windows)]
|
||||
fn is_ansi_code_supported(&self) -> bool {
|
||||
true
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use codex_terminal_detection::TerminalName;
|
||||
use crossterm::Command;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
use super::OscProtocol;
|
||||
use super::PostNotification;
|
||||
|
||||
#[test]
|
||||
fn post_notification_writes_plain_osc9_sequence() {
|
||||
let message = "a".repeat(2048);
|
||||
let mut ansi = String::new();
|
||||
let command = PostNotification {
|
||||
protocol: OscProtocol::Osc9,
|
||||
message: &message,
|
||||
terminal_name: TerminalName::Iterm2,
|
||||
dcs_passthrough: false,
|
||||
};
|
||||
|
||||
command
|
||||
.write_ansi(&mut ansi)
|
||||
.expect("OSC 9 command should format");
|
||||
|
||||
assert_eq!(ansi, format!("\u{1b}]9;{message}\u{7}"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn long_approval_in_ghostty_fits_the_notification_body() {
|
||||
// The next UTF-8 character crosses the cutoff for the ellipsis.
|
||||
let prefix = format!("Approval requested: {}", "a".repeat(231));
|
||||
let message = format!("{prefix}🦀{}", "b".repeat(2048));
|
||||
for (protocol, expected) in [
|
||||
(OscProtocol::Osc9, format!("\x1b]9;{prefix}…\x07")),
|
||||
(
|
||||
OscProtocol::Osc777,
|
||||
format!("\x1b]777;notify;Codex;{prefix}…\x1b\\"),
|
||||
),
|
||||
] {
|
||||
let mut ansi = String::new();
|
||||
PostNotification {
|
||||
protocol,
|
||||
message: &message,
|
||||
terminal_name: TerminalName::Ghostty,
|
||||
dcs_passthrough: false,
|
||||
}
|
||||
.write_ansi(&mut ansi)
|
||||
.expect("format Ghostty notification");
|
||||
assert_eq!(ansi, expected);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn post_notification_writes_tmux_dcs_wrapped_osc9_sequence() {
|
||||
let mut ansi = String::new();
|
||||
let command = PostNotification {
|
||||
protocol: OscProtocol::Osc9,
|
||||
message: "done",
|
||||
terminal_name: TerminalName::Iterm2,
|
||||
dcs_passthrough: true,
|
||||
};
|
||||
|
||||
command
|
||||
.write_ansi(&mut ansi)
|
||||
.expect("OSC 9 command should format");
|
||||
|
||||
assert_eq!(ansi, "\u{1b}Ptmux;\u{1b}\u{1b}]9;done\u{7}\u{1b}\\");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn post_notification_keeps_control_characters_out_of_osc9_body() {
|
||||
let mut ansi = String::new();
|
||||
let command = PostNotification {
|
||||
protocol: OscProtocol::Osc9,
|
||||
message: "before\x07\x1bafter",
|
||||
terminal_name: TerminalName::Iterm2,
|
||||
dcs_passthrough: true,
|
||||
};
|
||||
|
||||
command
|
||||
.write_ansi(&mut ansi)
|
||||
.expect("OSC 9 command should format");
|
||||
|
||||
assert_eq!(
|
||||
ansi,
|
||||
"\u{1b}Ptmux;\u{1b}\u{1b}]9;before after\u{7}\u{1b}\\"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn osc777_sends_title_and_plain_text_body_directly_and_through_tmux() {
|
||||
let mut sequences = Vec::new();
|
||||
for dcs_passthrough in [false, true] {
|
||||
let command = PostNotification {
|
||||
protocol: OscProtocol::Osc777,
|
||||
message: "Approval requested: cargo test; git clean\x07\x1b\u{009c}-fd",
|
||||
terminal_name: TerminalName::WindowsTerminal,
|
||||
dcs_passthrough,
|
||||
};
|
||||
let mut ansi = String::new();
|
||||
command.write_ansi(&mut ansi).expect("format OSC 777");
|
||||
sequences.push(ansi);
|
||||
}
|
||||
insta::assert_debug_snapshot!(sequences, @r#"
|
||||
[
|
||||
"\u{1b}]777;notify;Codex;Approval requested: cargo test; git clean -fd\u{1b}\\",
|
||||
"\u{1b}Ptmux;\u{1b}\u{1b}]777;notify;Codex;Approval requested: cargo test; git clean -fd\u{1b}\u{1b}\\\u{1b}\\",
|
||||
]
|
||||
"#);
|
||||
}
|
||||
}
|
||||
@@ -1,127 +0,0 @@
|
||||
use std::fmt;
|
||||
use std::io;
|
||||
use std::io::stdout;
|
||||
|
||||
use codex_terminal_detection::Multiplexer;
|
||||
use codex_terminal_detection::terminal_info;
|
||||
use crossterm::Command;
|
||||
use ratatui::crossterm::execute;
|
||||
|
||||
#[derive(Debug)]
|
||||
pub struct Osc9Backend {
|
||||
dcs_passthrough: bool,
|
||||
}
|
||||
|
||||
impl Default for Osc9Backend {
|
||||
fn default() -> Self {
|
||||
Self::new()
|
||||
}
|
||||
}
|
||||
|
||||
impl Osc9Backend {
|
||||
pub fn new() -> Self {
|
||||
Self {
|
||||
dcs_passthrough: matches!(terminal_info().multiplexer, Some(Multiplexer::Tmux { .. })),
|
||||
}
|
||||
}
|
||||
|
||||
pub fn notify(&mut self, message: &str) -> io::Result<()> {
|
||||
execute!(
|
||||
stdout(),
|
||||
PostNotification {
|
||||
message: message.to_string(),
|
||||
dcs_passthrough: self.dcs_passthrough,
|
||||
}
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
/// Command that emits an OSC 9 desktop notification with a message.
|
||||
#[derive(Debug, Clone)]
|
||||
pub struct PostNotification {
|
||||
pub message: String,
|
||||
pub dcs_passthrough: bool,
|
||||
}
|
||||
|
||||
impl Command for PostNotification {
|
||||
fn write_ansi(&self, f: &mut impl fmt::Write) -> fmt::Result {
|
||||
if self.dcs_passthrough {
|
||||
let escaped_message = escape_tmux_dcs_passthrough_payload(&self.message);
|
||||
write!(f, "\x1bPtmux;\x1b\x1b]9;{escaped_message}\x07\x1b\\")
|
||||
} else {
|
||||
write!(f, "\x1b]9;{}\x07", self.message)
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(windows)]
|
||||
fn execute_winapi(&self) -> io::Result<()> {
|
||||
Err(std::io::Error::other(
|
||||
"tried to execute PostNotification using WinAPI; use ANSI instead",
|
||||
))
|
||||
}
|
||||
|
||||
#[cfg(windows)]
|
||||
fn is_ansi_code_supported(&self) -> bool {
|
||||
true
|
||||
}
|
||||
}
|
||||
|
||||
fn escape_tmux_dcs_passthrough_payload(message: &str) -> String {
|
||||
message.replace('\u{1b}', "\u{1b}\u{1b}")
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use crossterm::Command;
|
||||
use pretty_assertions::assert_eq;
|
||||
|
||||
use super::PostNotification;
|
||||
|
||||
#[test]
|
||||
fn post_notification_writes_plain_osc9_sequence() {
|
||||
let mut ansi = String::new();
|
||||
let command = PostNotification {
|
||||
message: "hello".to_string(),
|
||||
dcs_passthrough: false,
|
||||
};
|
||||
|
||||
command
|
||||
.write_ansi(&mut ansi)
|
||||
.expect("OSC 9 command should format");
|
||||
|
||||
assert_eq!(ansi, "\u{1b}]9;hello\u{7}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn post_notification_writes_tmux_dcs_wrapped_osc9_sequence() {
|
||||
let mut ansi = String::new();
|
||||
let command = PostNotification {
|
||||
message: "done".to_string(),
|
||||
dcs_passthrough: true,
|
||||
};
|
||||
|
||||
command
|
||||
.write_ansi(&mut ansi)
|
||||
.expect("OSC 9 command should format");
|
||||
|
||||
assert_eq!(ansi, "\u{1b}Ptmux;\u{1b}\u{1b}]9;done\u{7}\u{1b}\\");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn post_notification_escapes_escape_bytes_inside_tmux_payload() {
|
||||
let mut ansi = String::new();
|
||||
let command = PostNotification {
|
||||
message: "danger\u{1b}[31m".to_string(),
|
||||
dcs_passthrough: true,
|
||||
};
|
||||
|
||||
command
|
||||
.write_ansi(&mut ansi)
|
||||
.expect("OSC 9 command should format");
|
||||
|
||||
assert_eq!(
|
||||
ansi,
|
||||
"\u{1b}Ptmux;\u{1b}\u{1b}]9;danger\u{1b}\u{1b}[31m\u{7}\u{1b}\\"
|
||||
);
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user