Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions docs/next/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion docs/next/api/herdr-api.schema.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"$schema": "https://json-schema.org/draft/2020-12/schema",
"protocol": 19,
"protocol": 20,
"schema_version": 1,
"schemas": {
"error_response": {
Expand Down
3 changes: 2 additions & 1 deletion src/app/actions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 } => {
Expand Down
13 changes: 13 additions & 0 deletions src/app/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down
7 changes: 7 additions & 0 deletions src/client/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
}
Expand Down
3 changes: 3 additions & 0 deletions src/events.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> },
Expand Down
20 changes: 20 additions & 0 deletions src/ghostty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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)
}
Expand Down
1 change: 1 addition & 0 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ mod server;
mod session;
mod sound;
mod terminal;
mod terminal_effects;
mod terminal_modes;
mod terminal_notify;
mod terminal_theme;
Expand Down
56 changes: 56 additions & 0 deletions src/pane.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 }) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Bell events drop under pressure

When the bounded application-event channel is full, try_send discards the parsed terminal bell event, causing the outer terminal to miss the pane's audible or visual bell.

Knowledge Base Used:

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’m keeping this best-effort on the existing bounded application-event channel. Blocking the PTY reader when the UI loop is under pressure can stall terminal output or deadlock against pane work; adding a reliable side-effect queue would be a broader transport-policy change than BEL forwarding.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a sound tradeoff. PTY reader stalls would degrade the core terminal experience in a much more visible way than an occasional missed bell — and bells are inherently best-effort signals. The existing channel back-pressure semantics already encode that priority correctly.

The one low-cost improvement worth considering: log the drop so it's observable without adding any blocking:

Suggested change
if let Err(err) = events.try_send(AppEvent::TerminalBell { pane_id, count }) {
if let Err(err) = events.try_send(AppEvent::TerminalBell { pane_id, count }) {
tracing::debug!(pane_id = ?pane_id, count, ?err, "bell event dropped (channel full)");

That way the behavior stays non-blocking but the drop isn't completely silent during debugging. Otherwise the approach is justified.

warn!(
pane = pane_id.raw(),
count,
err = %err,
"failed to queue terminal bell"
);
}
}

fn publish_reported_cwd(
pane_id: PaneId,
cwd: std::path::PathBuf,
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
Expand Down
23 changes: 22 additions & 1 deletion src/pane/terminal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
Expand Down Expand Up @@ -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(),
Expand All @@ -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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -1332,6 +1336,7 @@ impl GhosttyPaneTerminal {
ProcessBytesResult {
request_render,
render_delay,
terminal_bells,
clipboard_writes,
reported_cwd,
terminal_responses,
Expand Down Expand Up @@ -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);
Expand All @@ -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());
Expand Down
17 changes: 16 additions & 1 deletion src/protocol/wire.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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,
},
}

// ---------------------------------------------------------------------------
Expand Down Expand Up @@ -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]
Expand Down
Loading
Loading