fix: forward pane terminal bells (#2498)

refs #2453

Co-authored-by: akbash-bot <300245827+akbash-bot@users.noreply.github.com>
This commit is contained in:
akbash 2026-08-08 02:15:56 +03:00 committed by GitHub
parent 50ddc06f00
commit 6f311498ae
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
16 changed files with 255 additions and 12 deletions

View File

@ -8,10 +8,14 @@
- Settings and `ui.status_indicators = "symbols"` can now use distinct static shapes for blocked, working, done, idle, and unknown agent states. (#2260)
- The plugin marketplace now discovers valid manifests at repository roots and subdirectories, groups multiple plugins under each repository, and publishes their versions and exact default-branch commits.
### Changed
- Bumped the client/server protocol version to 20 for pane terminal bell forwarding.
### Fixed
- `herdr config check` now reports unknown built-in theme names instead of silently accepting them. (#2452)
- macOS `herdr --remote` clients now keep the accepted bridge socket blocking, preventing an immediate disconnect after the protocol handshake. (#2478, thanks @mathijshenquet)
- Prefix keybindings now preserve Shift in WezTerm Kitty keyboard mode, so commands such as config reload no longer trigger their unshifted action. (#2435)
- BEL characters emitted by pane programs now reach the outer terminal so its audible and visual bell settings can react. (#2453)
- Stable direct installs, self-updates, and remote helper downloads now require and verify the SHA-256 digest published for each GitHub release asset.
- Configs containing the retired Herdr-written `ui.agent_panel_scope` setting no longer report it as an unknown key after upgrades. (#2292)
- Claude Code confirmation prompts using `Enter to confirm · Esc to cancel` now report `blocked` instead of `idle`. (#2268)

View File

@ -1,6 +1,6 @@
{
"$schema": "https://json-schema.org/draft/2020-12/schema",
"protocol": 19,
"protocol": 20,
"schema_version": 1,
"schemas": {
"error_response": {

View File

@ -2902,9 +2902,10 @@ impl AppState {
.collect()
}
}
// Both intercepted before this dispatch — in App::handle_internal_event (monolithic)
// Intercepted before this dispatch — in App::handle_internal_event (monolithic)
// or via HeadlessServer forwarding to the foreground client (server); never touch
// AppState. Kept for AppEvent exhaustiveness.
AppEvent::TerminalBell { .. } => Vec::new(),
AppEvent::ClipboardWrite { .. } => Vec::new(),
AppEvent::PrefixInputSource { .. } => Vec::new(),
AppEvent::TerminalCwdReported { pane_id, cwd } => {

View File

@ -64,6 +64,10 @@ impl App {
results,
cache_updates,
} => self.handle_git_status_refreshed(results, cache_updates),
ev @ AppEvent::TerminalBell { .. } => {
self.handle_internal_event(ev);
false
}
ev => {
self.handle_internal_event(ev);
true
@ -97,6 +101,15 @@ impl App {
}
pub(crate) fn handle_internal_event(&mut self, ev: AppEvent) {
if let AppEvent::TerminalBell { count, .. } = ev {
if let Err(err) =
crate::terminal_effects::write_terminal_bells(&mut std::io::stdout(), count)
{
tracing::warn!(err = %err, "failed to emit terminal bell");
}
return;
}
if let AppEvent::ClipboardWrite { content } = ev {
#[cfg(not(test))]
crate::selection::write_osc52_bytes(&content);

View File

@ -1571,6 +1571,13 @@ async fn run_client_loop(
let _ = stdout.flush();
}
}
ServerMessage::TerminalBell { count } => {
if let Err(err) =
crate::terminal_effects::write_terminal_bells(&mut io::stdout(), count)
{
warn!(err = %err, "failed to emit terminal bell");
}
}
ServerMessage::ServerShutdown { reason } => {
return Err(ClientError::ServerShutdown { reason });
}

View File

@ -124,6 +124,9 @@ pub enum AppEvent {
updated: Vec<crate::detect::manifest_update::ManifestUpdateCommit>,
status: crate::detect::manifest_update::ManifestUpdateStatus,
},
/// A pane child emitted one or more executable BEL characters.
/// The host-facing process forwards them to its outer terminal.
TerminalBell { pane_id: PaneId, count: u16 },
/// A pane child emitted a valid OSC 52 clipboard write. The main loop
/// re-emits it through herdr's own clipboard writer.
ClipboardWrite { content: Vec<u8> },

View File

@ -470,12 +470,22 @@ const MAX_CLIPBOARD_BYTES: usize = 192 * 1024;
#[derive(Default)]
struct TerminalCallbackState {
write_pty: Option<Box<WritePtyCallback>>,
bell_count: u16,
pwd_changes: Vec<Vec<u8>>,
clipboard_writes: Vec<Vec<u8>>,
size_report: ffi::GhosttySizeReportSize,
color_scheme: Option<ColorScheme>,
}
unsafe extern "C" fn bell_trampoline(_terminal: ffi::GhosttyTerminal, userdata: *mut c_void) {
if userdata.is_null() {
return;
}
// SAFETY: userdata is the TerminalCallbackState installed with this terminal.
let state = unsafe { &mut *userdata.cast::<TerminalCallbackState>() };
state.bell_count = state.bell_count.saturating_add(1);
}
unsafe extern "C" fn color_scheme_trampoline(
_terminal: ffi::GhosttyTerminal,
userdata: *mut c_void,
@ -820,6 +830,12 @@ impl Terminal {
(size_trampoline as *const ()).cast(),
)
.into_result()?;
ffi::ghostty_terminal_set(
terminal.raw,
ffi::GhosttyTerminalOption_GHOSTTY_TERMINAL_OPT_BELL,
(bell_trampoline as *const ()).cast(),
)
.into_result()?;
ffi::ghostty_terminal_set(
terminal.raw,
ffi::GhosttyTerminalOption_GHOSTTY_TERMINAL_OPT_PWD_CHANGED,
@ -979,6 +995,10 @@ impl Terminal {
mem::replace(&mut self.callback_state.color_scheme, color_scheme)
}
pub fn take_bell_count(&mut self) -> u16 {
mem::take(&mut self.callback_state.bell_count)
}
pub fn take_pwd_changes(&mut self) -> Vec<Vec<u8>> {
mem::take(&mut self.callback_state.pwd_changes)
}

View File

@ -93,6 +93,7 @@ mod server;
mod session;
mod sound;
mod terminal;
mod terminal_effects;
mod terminal_modes;
mod terminal_notify;
mod terminal_theme;

View File

@ -1499,6 +1499,20 @@ fn usable_reported_cwd(cwd: std::path::PathBuf) -> Option<std::path::PathBuf> {
(cwd.is_absolute() && cwd.is_dir()).then_some(cwd)
}
fn publish_terminal_bells(pane_id: PaneId, count: u16, events: &mpsc::Sender<AppEvent>) {
if count == 0 {
return;
}
if let Err(err) = events.try_send(AppEvent::TerminalBell { pane_id, count }) {
warn!(
pane = pane_id.raw(),
count,
err = %err,
"failed to queue terminal bell"
);
}
}
fn publish_reported_cwd(
pane_id: PaneId,
cwd: std::path::PathBuf,
@ -1868,6 +1882,7 @@ impl PaneRuntime {
let shell_pid = child_pid.load(Ordering::Acquire);
let result =
terminal.process_pty_bytes(pane_id, shell_pid, bytes, &response_writer);
publish_terminal_bells(pane_id, result.terminal_bells, &read_events);
observe_detection_content_change(bytes, &detection_content_seq);
if result.request_render && render_dirty.request_pty(pane_id) {
render_notify.notify_one();
@ -2028,6 +2043,7 @@ impl PaneRuntime {
let shell_pid = child_pid.load(Ordering::Acquire);
let result =
terminal.process_pty_bytes(pane_id, shell_pid, bytes, &response_writer);
publish_terminal_bells(pane_id, result.terminal_bells, &events);
if agent_detection == AgentDetection::Enabled {
observe_detection_content_change(bytes, &detection_content_seq);
}
@ -4214,6 +4230,46 @@ mod tests {
.expect("re-entering active authority should notify detection reset");
}
#[cfg(unix)]
#[tokio::test]
async fn spawned_pty_reader_aggregates_terminal_bells() {
let (events, mut event_rx) = mpsc::channel(8);
let pane_id = PaneId::from_raw(42);
let runtime = PaneRuntime::spawn_shell_command(
pane_id,
24,
80,
std::env::temp_dir(),
"printf '\\a\\a'; sleep 0.05",
&PaneLaunchEnv::default(),
AgentDetection::Disabled,
0,
crate::terminal_theme::TerminalTheme::default(),
None,
events,
Arc::new(Notify::new()),
Arc::new(RenderSignal::new()),
)
.unwrap();
let bell = tokio::time::timeout(std::time::Duration::from_secs(2), async {
loop {
if let Some(AppEvent::TerminalBell {
pane_id: delivered_pane,
count,
}) = event_rx.recv().await
{
break (delivered_pane, count);
}
}
})
.await
.expect("PTY reader should publish terminal bells");
assert_eq!(bell, (pane_id, 2));
runtime.shutdown();
}
#[tokio::test]
async fn state_changed_event_waits_for_queue_space_instead_of_dropping() {
let (tx, mut rx) = mpsc::channel(1);

View File

@ -145,6 +145,7 @@ impl InputState {
pub(crate) struct ProcessBytesResult {
pub request_render: bool,
pub render_delay: Option<Duration>,
pub terminal_bells: u16,
pub clipboard_writes: Vec<Vec<u8>>,
pub reported_cwd: Option<std::path::PathBuf>,
pub terminal_responses: Vec<Bytes>,
@ -1201,6 +1202,7 @@ impl GhosttyPaneTerminal {
return ProcessBytesResult {
request_render: false,
render_delay: None,
terminal_bells: 0,
clipboard_writes: Vec::new(),
reported_cwd: None,
terminal_responses: Vec::new(),
@ -1209,7 +1211,8 @@ impl GhosttyPaneTerminal {
let _ = core.terminal.take_pwd_changes();
// Restored history may have exercised terminal callbacks before this live PTY write.
// Those writes must not be delivered as live pane output.
// Those effects must not be delivered as live pane output.
let _ = core.terminal.take_bell_count();
let _ = core.terminal.take_clipboard_writes();
let default_color_observation = core.default_color_tracker.observe(bytes);
if shell_pid > 0 && default_color_observation {
@ -1276,6 +1279,7 @@ impl GhosttyPaneTerminal {
xtgettcap_responses,
&mut terminal_responses,
);
let terminal_bells = core.terminal.take_bell_count();
let clipboard_writes = core.terminal.take_clipboard_writes();
let reported_cwd = core
.terminal
@ -1332,6 +1336,7 @@ impl GhosttyPaneTerminal {
ProcessBytesResult {
request_render,
render_delay,
terminal_bells,
clipboard_writes,
reported_cwd,
terminal_responses,
@ -3734,6 +3739,21 @@ mod tests {
);
}
#[test]
fn process_pty_bytes_surfaces_live_bells_only() {
let (tx, _rx) = mpsc::channel(4);
let terminal = crate::ghostty::Terminal::new(80, 24, 100).unwrap();
let pane = GhosttyPaneTerminal::new(terminal, tx.clone()).unwrap();
let pane_id = PaneId::from_raw(1);
pane.seed_history_ansi("stale\x07");
let result = pane.process_pty_bytes(pane_id, 0, b"\x07\x1b]0;title\x07\x07", &tx);
assert_eq!(result.terminal_bells, 2);
let drained = pane.process_pty_bytes(pane_id, 0, b"live output", &tx);
assert_eq!(drained.terminal_bells, 0);
}
#[test]
fn process_pty_bytes_surfaces_clipboard_writes_without_other_results() {
let (tx, _rx) = mpsc::channel(4);
@ -3749,6 +3769,7 @@ mod tests {
assert!(result.request_render);
assert_eq!(result.render_delay, None);
assert_eq!(result.terminal_bells, 0);
assert_eq!(result.clipboard_writes, vec![b"clipboard".to_vec()]);
assert_eq!(result.reported_cwd, None);
assert!(result.terminal_responses.is_empty());

View File

@ -13,7 +13,7 @@ use serde::{Deserialize, Serialize};
// ---------------------------------------------------------------------------
/// Current protocol version. Bumped when wire format changes incompatibly.
pub const PROTOCOL_VERSION: u32 = 19;
pub const PROTOCOL_VERSION: u32 = 20;
/// Maximum allowed frame payload size (2 MB). Frames larger than this are
/// rejected to prevent denial-of-service via oversized length prefixes.
@ -711,6 +711,12 @@ pub enum ServerMessage {
/// Whether the ASCII input source should be active.
active: bool,
},
/// Ring the foreground client's outer terminal for pane-originated BEL characters.
TerminalBell {
/// Number of BEL characters parsed from one PTY read.
count: u16,
},
}
// ---------------------------------------------------------------------------
@ -1555,6 +1561,15 @@ mod tests {
}
}
#[test]
fn server_terminal_bell_roundtrip() {
let msg = ServerMessage::TerminalBell { count: 3 };
let encoded = bincode::serde::encode_to_vec(&msg, bincode::config::standard()).unwrap();
let (decoded, _): (ServerMessage, _) =
bincode::serde::decode_from_slice(&encoded, bincode::config::standard()).unwrap();
assert_eq!(msg, decoded);
}
// ---- Framing ----
#[test]

View File

@ -2098,6 +2098,15 @@ impl HeadlessServer {
/// Returns true if the event changed visual state (requiring a re-render).
fn handle_internal_event_with_forwarding(&mut self, ev: AppEvent) -> bool {
match &ev {
AppEvent::TerminalBell { pane_id, count } => {
if !self.send_to_foreground_client(ServerMessage::TerminalBell { count: *count }) {
debug!(
pane = pane_id.raw(),
count, "dropped terminal bell without a foreground client"
);
}
false
}
AppEvent::ClipboardWrite { content } => {
// Clipboard writes are client-local side effects. Forward them only to
// the foreground client instead of broadcasting to every attached client.
@ -9583,6 +9592,72 @@ next_tab = ""
assert!(!server.app.state.request_client_config_reload);
}
#[test]
fn terminal_bell_targets_foreground_client_only() {
let mut server = test_headless_server();
let (background_tx, background_control_rx, _background_rx) = test_client_writer();
let (foreground_tx, foreground_control_rx, _foreground_rx) = test_client_writer();
server.clients.insert(
1,
ClientConnection::new(
(120, 40),
crate::kitty_graphics::HostCellSize::default(),
crate::terminal_theme::TerminalTheme::default(),
None,
1,
RenderEncoding::SemanticFrame,
Some(background_tx),
),
);
server.clients.insert(
2,
ClientConnection::new(
(80, 24),
crate::kitty_graphics::HostCellSize::default(),
crate::terminal_theme::TerminalTheme::default(),
None,
2,
RenderEncoding::SemanticFrame,
Some(foreground_tx),
),
);
server.foreground_client_id = Some(2);
let changed = server.handle_internal_event_with_forwarding(AppEvent::TerminalBell {
pane_id: crate::layout::PaneId::from_raw(1),
count: 3,
});
assert!(!changed);
match read_server_message(
foreground_control_rx
.recv_timeout(Duration::from_millis(100))
.expect("foreground terminal bell message"),
) {
ServerMessage::TerminalBell { count } => assert_eq!(count, 3),
other => panic!("expected terminal bell message, got {other:?}"),
}
assert!(
background_control_rx
.recv_timeout(Duration::from_millis(50))
.is_err(),
"background client should not receive terminal bells"
);
server.foreground_client_id = None;
server.handle_internal_event_with_forwarding(AppEvent::TerminalBell {
pane_id: crate::layout::PaneId::from_raw(1),
count: 1,
});
assert!(
foreground_control_rx
.recv_timeout(Duration::from_millis(50))
.is_err(),
"bells without a foreground client must not be retained"
);
}
#[test]
fn clipboard_write_targets_foreground_client_only() {
let mut server = test_headless_server();

27
src/terminal_effects.rs Normal file
View File

@ -0,0 +1,27 @@
use std::io::{self, Write};
const BELL_CHUNK: [u8; 64] = [b'\x07'; 64];
pub(crate) fn write_terminal_bells<W: Write>(writer: &mut W, count: u16) -> io::Result<()> {
let full_chunks = usize::from(count) / BELL_CHUNK.len();
let remainder = usize::from(count) % BELL_CHUNK.len();
for _ in 0..full_chunks {
writer.write_all(&BELL_CHUNK)?;
}
writer.write_all(&BELL_CHUNK[..remainder])?;
writer.flush()
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn writes_exact_terminal_bell_count() {
let mut output = Vec::new();
write_terminal_bells(&mut output, 130).unwrap();
assert_eq!(output, vec![b'\x07'; 130]);
}
}

View File

@ -304,7 +304,7 @@ fn ping_over_socket_returns_version() {
assert_eq!(value["result"]["version"], env!("CARGO_PKG_VERSION"));
// Intentionally hardcoded so wire protocol bumps require updating this test.
// Changing this value means old clients/servers are no longer compatible.
assert_eq!(value["result"]["protocol"], 19);
assert_eq!(value["result"]["protocol"], 20);
cleanup_spawned_herdr(child, base);
}

View File

@ -389,7 +389,7 @@ fn status_commands_report_client_and_server_versions() {
"stdout: {full_stdout}"
);
assert!(
full_stdout.contains(" protocol: 19"),
full_stdout.contains(" protocol: 20"),
"stdout: {full_stdout}"
);
assert!(full_stdout.contains("server:\n"), "stdout: {full_stdout}");
@ -422,7 +422,7 @@ fn status_commands_report_client_and_server_versions() {
"stdout: {server_stdout}"
);
assert!(
server_stdout.contains("protocol: 19"),
server_stdout.contains("protocol: 20"),
"stdout: {server_stdout}"
);
@ -434,7 +434,7 @@ fn status_commands_report_client_and_server_versions() {
"stdout: {client_stdout}"
);
assert!(
client_stdout.contains("protocol: 19"),
client_stdout.contains("protocol: 20"),
"stdout: {client_stdout}"
);
assert!(
@ -444,7 +444,7 @@ fn status_commands_report_client_and_server_versions() {
let full_json = run_cli_json(&socket_path, &["status", "--json"]);
assert_eq!(full_json["client"]["version"], env!("CARGO_PKG_VERSION"));
assert_eq!(full_json["client"]["protocol"], 19);
assert_eq!(full_json["client"]["protocol"], 20);
assert_eq!(full_json["server"]["status"], "running");
assert_eq!(full_json["server"]["running"], true);
assert_eq!(full_json["server"]["compatible"], true);
@ -458,12 +458,12 @@ fn status_commands_report_client_and_server_versions() {
let server_json = run_cli_json(&socket_path, &["status", "server", "--json"]);
assert_eq!(server_json["status"], "running");
assert_eq!(server_json["version"], env!("CARGO_PKG_VERSION"));
assert_eq!(server_json["protocol"], 19);
assert_eq!(server_json["protocol"], 20);
assert_eq!(server_json["compatible"], true);
let client_json = run_cli_json(&socket_path, &["status", "client", "--json"]);
assert_eq!(client_json["version"], env!("CARGO_PKG_VERSION"));
assert_eq!(client_json["protocol"], 19);
assert_eq!(client_json["protocol"], 20);
assert!(client_json["binary"]
.as_str()
.is_some_and(|path| !path.is_empty()));

View File

@ -15,7 +15,7 @@ static INIT: Once = Once::new();
static CLEANUP_GUARD: OnceLock<CleanupGuard> = OnceLock::new();
const WATCHDOG_SCAN_INTERVAL: Duration = Duration::from_secs(1);
const RUNTIME_OWNER_MARKER: &str = ".herdr-test-owner-pid";
pub const CURRENT_PROTOCOL: u32 = 19;
pub const CURRENT_PROTOCOL: u32 = 20;
pub fn register_spawned_herdr_pid(pid: Option<u32>) {
let Some(pid) = pid else {