From 3636908bb9537ab7789418b07880c051c547de68 Mon Sep 17 00:00:00 2001 From: Hunter Bown Date: Mon, 4 May 2026 12:49:03 -0500 Subject: [PATCH 1/2] fix(notifications): default Windows Auto fallback to Off, not BEL MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On Windows, the audio stack maps BEL (`\x07`) to the `SystemAsterisk` / `MB_OK` chime — the same sound applications use for error popups. So with the previous `Method::Auto` fallback to `Bel`, every successful turn-completion notification ended up sounding identical to a software error. Reported by a community user who described it as "the popup-error sound from a CAD program I used to use" (#583). resolve_method() now returns `Off` instead of `Bel` on Windows for unknown TERM_PROGRAM values. Known OSC-9-capable terminals (`iTerm.app`, `Ghostty`, `WezTerm`) still resolve to `Osc9` on every platform, so users running WezTerm on Windows keep getting real notifications. macOS and Linux behaviour is unchanged. Windows users who actively want an audible cue can opt back in by setting `[notifications].method = "bel"` in `~/.deepseek/config.toml`. Also: - Documents `[notifications]` in `docs/CONFIGURATION.md` with an explicit Windows note (the schema was previously undocumented). - Updates the inline comment in `config.example.toml` so users reading the seed config see the platform-specific behaviour. - Splits the existing `auto_detect_picks_bel_for_unknown` test into a Unix variant (`#[cfg(not(target_os = "windows"))]`) and adds a new Windows-gated test that asserts the `Off` fallback, so CI's Windows runner exercises the platform-specific path. Co-Authored-By: Claude Opus 4.7 (1M context) --- config.example.toml | 7 ++++- crates/tui/src/config.rs | 8 ++++- crates/tui/src/tui/notifications.rs | 49 ++++++++++++++++++++++++++--- docs/CONFIGURATION.md | 20 ++++++++++++ 4 files changed, 78 insertions(+), 6 deletions(-) diff --git a/config.example.toml b/config.example.toml index 280f0520..a40e59e3 100644 --- a/config.example.toml +++ b/config.example.toml @@ -285,7 +285,12 @@ default_text_model = "deepseek-ai/deepseek-v4-pro" # than `threshold_secs`. Useful when you tab away from the TUI and want an alert. # # method = "auto" # auto | osc9 | bel | off -# auto: OSC 9 for iTerm.app / Ghostty / WezTerm, BEL otherwise. +# auto: OSC 9 for iTerm.app / Ghostty / WezTerm. +# On macOS / Linux, falls back to BEL. +# On Windows, falls back to "off" — BEL maps to the +# system error chime (SystemAsterisk / MB_OK), which +# sounds like an error popup. Set method = "bel" +# explicitly to opt back in (#583). # osc9: \x1b]9;\x07 (iTerm2-style; shows macOS notification) # bel: plain \x07 beep # off: disable entirely diff --git a/crates/tui/src/config.rs b/crates/tui/src/config.rs index a57d27d9..0431bb42 100644 --- a/crates/tui/src/config.rs +++ b/crates/tui/src/config.rs @@ -349,7 +349,9 @@ pub struct TuiConfig { #[derive(Debug, Clone, Deserialize, Default, PartialEq, Eq)] #[serde(rename_all = "kebab-case")] pub enum NotificationMethod { - /// Auto-detect: OSC 9 for iTerm.app / Ghostty / WezTerm; BEL otherwise. + /// Auto-detect: OSC 9 for iTerm.app / Ghostty / WezTerm; BEL on + /// macOS / Linux otherwise; on Windows the fallback is `Off` + /// because BEL maps to the system error chime there (#583). #[default] Auto, /// OSC 9 escape. @@ -368,6 +370,10 @@ fn default_threshold_secs() -> u64 { #[derive(Debug, Clone, Deserialize, Default)] pub struct NotificationsConfig { /// Delivery method: `auto` | `osc9` | `bel` | `off`. Default: `auto`. + /// `auto` resolves to OSC 9 in iTerm.app / Ghostty / WezTerm; on + /// macOS / Linux it falls back to BEL, and on Windows it falls + /// back to `Off` so the post-turn notification doesn't ring the + /// system error chime (#583). #[serde(default)] pub method: NotificationMethod, /// Only notify when the turn took at least this many seconds. Default: 30. diff --git a/crates/tui/src/tui/notifications.rs b/crates/tui/src/tui/notifications.rs index d42df7e8..944c9273 100644 --- a/crates/tui/src/tui/notifications.rs +++ b/crates/tui/src/tui/notifications.rs @@ -12,7 +12,13 @@ use std::time::Duration; #[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] pub enum Method { /// Automatically pick `Osc9` for known capable terminals - /// (`iTerm.app`, `Ghostty`, `WezTerm`) and fall back to `Bel` otherwise. + /// (`iTerm.app`, `Ghostty`, `WezTerm`); fall back to `Bel` on + /// macOS / Linux. On Windows the fallback is `Off` instead of + /// `Bel`, because the OS audio stack maps `\x07` to the + /// `SystemAsterisk` / `MB_OK` chime — the same sound used by + /// application error popups (#583). Windows users who want an + /// audible cue can opt in by setting + /// `[notifications].method = "bel"` explicitly. #[default] Auto, /// OSC 9 escape: `\x1b]9;\x07` @@ -38,13 +44,24 @@ impl Method { /// Resolve `Auto` to a concrete method by inspecting `$TERM_PROGRAM`. /// -/// Known OSC-9 capable programs: `iTerm.app`, `Ghostty`, `WezTerm`. -/// Everything else falls back to `Bel`. +/// Known OSC-9 capable programs: `iTerm.app`, `Ghostty`, `WezTerm` +/// (these resolve to `Osc9` on every platform, including Windows +/// when running inside WezTerm). +/// +/// Otherwise the fallback is platform-dependent: +/// - **macOS / Linux / other Unix:** `Bel` (a single `\x07` byte). +/// - **Windows:** `Off`. BEL is mapped by the Windows audio stack +/// to `SystemAsterisk` / `MB_OK`, the same chime used by +/// application error popups, so it sounds like an error +/// notification even though the turn completed successfully (#583). +/// Users can opt back in with `[notifications].method = "bel"` or +/// pick a known OSC-9 terminal. #[must_use] fn resolve_method() -> Method { let term_program = std::env::var("TERM_PROGRAM").unwrap_or_default(); match term_program.as_str() { "iTerm.app" | "Ghostty" | "WezTerm" => Method::Osc9, + _ if cfg!(target_os = "windows") => Method::Off, _ => Method::Bel, } } @@ -274,7 +291,8 @@ mod tests { } #[test] - fn auto_detect_picks_bel_for_unknown() { + #[cfg(not(target_os = "windows"))] + fn auto_detect_picks_bel_for_unknown_on_unix() { let _lock = env_lock(); let prev = std::env::var_os("TERM_PROGRAM"); // SAFETY: test-only; serialised by env_lock(). @@ -290,6 +308,29 @@ mod tests { assert_eq!(resolved, Method::Bel); } + /// #583: on Windows, an unknown TERM_PROGRAM resolves to `Off` + /// (not `Bel`) so the post-turn notification doesn't ring the + /// `SystemAsterisk` / `MB_OK` chime. Known OSC-9 terminals like + /// WezTerm still resolve to `Osc9` — see the iTerm test, which + /// also exercises the OSC-9 branch on Windows. + #[test] + #[cfg(target_os = "windows")] + fn auto_detect_picks_off_for_unknown_on_windows() { + let _lock = env_lock(); + let prev = std::env::var_os("TERM_PROGRAM"); + // SAFETY: test-only; serialised by env_lock(). + unsafe { std::env::set_var("TERM_PROGRAM", "Windows Terminal") }; + let resolved = resolve_method(); + // SAFETY: test-only; serialised by env_lock(). + unsafe { + match prev { + Some(v) => std::env::set_var("TERM_PROGRAM", v), + None => std::env::remove_var("TERM_PROGRAM"), + } + } + assert_eq!(resolved, Method::Off); + } + #[test] fn humanize_duration_seconds_and_minutes() { assert_eq!(humanize_duration(Duration::from_secs(0)), "0s"); diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index f5aea250..2d8fe52f 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -400,6 +400,26 @@ Notes: - See [`MEMORY.md`](MEMORY.md) for examples and the full `/memory` command surface. +### Notifications + +The TUI can emit a desktop notification (OSC 9 escape or plain BEL) when a turn takes longer than a threshold, so you can tab away while a long task runs. Configuration lives under `[notifications]`: + +```toml +[notifications] +method = "auto" # auto | osc9 | bel | off +threshold_secs = 30 # only notify when the turn took >= this many seconds +include_summary = false # include elapsed time + cost in the notification body +``` + +Method semantics: + +- `auto` (default) — picks `osc9` for `iTerm.app`, `Ghostty`, and `WezTerm` (detected via `$TERM_PROGRAM`). On macOS and Linux it falls back to `bel`. **On Windows the fallback is `off`** instead of `bel`, because the Windows audio stack maps `\x07` to the `SystemAsterisk` / `MB_OK` chime — the same sound application error popups use, so a successful-turn notification ends up sounding like an error (#583). +- `osc9` — emit `\x1b]9;\x07`. Inside tmux the sequence is wrapped in DCS passthrough so it reaches the outer terminal. +- `bel` — emit a single `\x07` byte. Use this on Windows only if you actively want the chime back. +- `off` — disable post-turn notifications entirely. + +Windows users who run inside a known OSC-9 terminal (e.g. WezTerm on Windows) keep getting OSC-9 notifications; the `off` fallback only applies when no recognised `TERM_PROGRAM` is detected. + ### Parsed but currently unused (reserved for future versions) These keys are accepted by the config loader but not currently used by the interactive TUI or built-in tools: From a68c8dc974a99ef842ff065b99b8c3e85b6a50dd Mon Sep 17 00:00:00 2001 From: Hunter Bown Date: Mon, 4 May 2026 13:38:21 -0500 Subject: [PATCH 2/2] docs(notifications): only completed turns notify; add Key Reference + WezTerm-on-Windows test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Post-merge review feedback on #583 surfaced four small accuracy gaps: 1. The narrative docs in `docs/CONFIGURATION.md` and the inline comment in `config.example.toml` said the notification fires "when a turn takes longer than a threshold" — but the call site in `tui/ui.rs:928` is gated on `TurnOutcomeStatus::Completed`. Failed and cancelled turns are silent on purpose. Spell that out so users don't expect alerts on long failures. 2. The `notify_done` rustdoc still summarised `Auto` as "Osc9 for known terminals, Bel otherwise" — internally inconsistent with the new Windows-aware fallback documented one screen earlier on the `Method::Auto` enum and on `resolve_method`. Update the public rustdoc to point at the canonical resolution table on `resolve_method` and call out the `Off`-on-Windows branch. 3. The `## Key Reference` list in `docs/CONFIGURATION.md` had no entries for `[notifications].method`, `[notifications].threshold_secs`, or `[notifications].include_summary`. Other features with a dedicated subsection (e.g. `[memory].enabled`) are listed there too, so readers scanning the canonical key list could not discover the notification knobs. Added the three keys with cross-references to the Notifications subsection. 4. The Windows-only test only covered the unknown-`TERM_PROGRAM` → `Off` fallback. The positive path (known OSC-9 terminal still resolves to `Osc9`) was only tested via `iTerm.app`, which is a macOS-only program — Windows CI would still pass if the `WezTerm` arm of the match disappeared. Added `auto_detect_picks_osc9_for_wezterm_on_windows` so the WezTerm-on-Windows compatibility guarantee is exercised on the Windows runner. Co-Authored-By: Claude Opus 4.7 (1M context) --- config.example.toml | 6 +++-- crates/tui/src/tui/notifications.rs | 39 ++++++++++++++++++++++++----- docs/CONFIGURATION.md | 16 +++++++++++- 3 files changed, 52 insertions(+), 9 deletions(-) diff --git a/config.example.toml b/config.example.toml index a40e59e3..6f097e7a 100644 --- a/config.example.toml +++ b/config.example.toml @@ -281,8 +281,10 @@ default_text_model = "deepseek-ai/deepseek-v4-pro" # ───────────────────────────────────────────────────────────────────────────────── # Desktop Notifications (OSC 9 / BEL on long agent-turn completion) # ───────────────────────────────────────────────────────────────────────────────── -# Emits an escape sequence to the terminal when a turn finishes and took longer -# than `threshold_secs`. Useful when you tab away from the TUI and want an alert. +# Emits an escape sequence to the terminal when a turn **completes successfully** +# and took longer than `threshold_secs`. Failed or cancelled turns are +# intentionally silent. Useful when you tab away from the TUI and want an alert +# for "your task is ready". # # method = "auto" # auto | osc9 | bel | off # auto: OSC 9 for iTerm.app / Ghostty / WezTerm. diff --git a/crates/tui/src/tui/notifications.rs b/crates/tui/src/tui/notifications.rs index 944c9273..df7992ed 100644 --- a/crates/tui/src/tui/notifications.rs +++ b/crates/tui/src/tui/notifications.rs @@ -122,9 +122,13 @@ pub fn notify_done_to( /// Emit a turn-complete notification to **stdout** if `elapsed >= threshold`. /// -/// With `method = Auto`, selects `Osc9` for known capable terminals and `Bel` -/// otherwise. Pass `in_tmux = true` (i.e. `$TMUX` is non-empty at runtime) -/// to wrap OSC 9 in a DCS passthrough. +/// With `method = Auto`, selects `Osc9` for known capable terminals +/// (`iTerm.app`, `Ghostty`, `WezTerm`); the unknown-terminal fallback is +/// platform-aware — `Bel` on macOS / Linux, `Off` on Windows (where BEL +/// maps to the `SystemAsterisk` / `MB_OK` error chime, #583). See +/// [`resolve_method`] for the canonical resolution table. Pass +/// `in_tmux = true` (i.e. `$TMUX` is non-empty at runtime) to wrap OSC 9 +/// in a DCS passthrough. pub fn notify_done( method: Method, in_tmux: bool, @@ -310,9 +314,7 @@ mod tests { /// #583: on Windows, an unknown TERM_PROGRAM resolves to `Off` /// (not `Bel`) so the post-turn notification doesn't ring the - /// `SystemAsterisk` / `MB_OK` chime. Known OSC-9 terminals like - /// WezTerm still resolve to `Osc9` — see the iTerm test, which - /// also exercises the OSC-9 branch on Windows. + /// `SystemAsterisk` / `MB_OK` chime. #[test] #[cfg(target_os = "windows")] fn auto_detect_picks_off_for_unknown_on_windows() { @@ -331,6 +333,31 @@ mod tests { assert_eq!(resolved, Method::Off); } + /// #583: known OSC-9 terminals must still resolve to `Osc9` on + /// Windows — the off-fallback only applies to unrecognised + /// `TERM_PROGRAM`. The cross-platform iTerm test above is a thin + /// proxy because iTerm itself only runs on macOS; if the WezTerm + /// arm of the match silently disappeared, that test would still + /// pass on the Windows runner and we'd lose the WezTerm-on-Windows + /// compatibility guarantee. Pin it directly. + #[test] + #[cfg(target_os = "windows")] + fn auto_detect_picks_osc9_for_wezterm_on_windows() { + let _lock = env_lock(); + let prev = std::env::var_os("TERM_PROGRAM"); + // SAFETY: test-only; serialised by env_lock(). + unsafe { std::env::set_var("TERM_PROGRAM", "WezTerm") }; + let resolved = resolve_method(); + // SAFETY: test-only; serialised by env_lock(). + unsafe { + match prev { + Some(v) => std::env::set_var("TERM_PROGRAM", v), + None => std::env::remove_var("TERM_PROGRAM"), + } + } + assert_eq!(resolved, Method::Osc9); + } + #[test] fn humanize_duration_seconds_and_minutes() { assert_eq!(humanize_duration(Duration::from_secs(0)), "0s"); diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index 2d8fe52f..9768e5a5 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -368,6 +368,20 @@ If you are upgrading from older releases: - `[capacity].deepseek_v4_pro_prior` (float, default `3.5`) - `[capacity].deepseek_v4_flash_prior` (float, default `4.2`) - `[capacity].fallback_default_prior` (float, default `3.8`) +- `[notifications].method` (string, optional): `auto`, `osc9`, `bel`, or + `off`. Defaults to `auto`. The TUI fires this on completed (successful) + turns whose elapsed time meets `threshold_secs`; failed and cancelled + turns are silent. `auto` resolves to `osc9` for `iTerm.app`, `Ghostty`, + and `WezTerm` (detected via `$TERM_PROGRAM`). Otherwise the fallback is + `bel` on macOS / Linux and `off` on Windows (where BEL maps to the + system error chime — see the [Notifications](#notifications) section + for the full rationale, #583). +- `[notifications].threshold_secs` (int, optional): defaults to `30`. + Only completed turns whose elapsed time meets or exceeds this fire a + notification. +- `[notifications].include_summary` (bool, optional): defaults to + `false`. When `true`, the notification body includes the elapsed + duration and the turn's USD cost. - `tui.alternate_screen` (string, optional): `auto`, `always`, or `never`. `auto` disables the alternate screen in Zellij; `--no-alt-screen` forces inline mode. Set `never` or run with `--no-alt-screen` when you want real terminal scrollback. - `tui.mouse_capture` (bool, optional, default `true` when the alternate screen is active): enable internal mouse scrolling, transcript selection, and right-click context actions. TUI-owned drag selection copies only user/assistant transcript text. Set this to `false` or run with `--no-mouse-capture` for raw terminal selection. - `tui.terminal_probe_timeout_ms` (int, optional, default `500`): startup terminal-mode probe timeout in milliseconds. Values are clamped to `100..=5000`; timeout emits a warning and aborts startup instead of hanging indefinitely. @@ -402,7 +416,7 @@ Notes: ### Notifications -The TUI can emit a desktop notification (OSC 9 escape or plain BEL) when a turn takes longer than a threshold, so you can tab away while a long task runs. Configuration lives under `[notifications]`: +The TUI can emit a desktop notification (OSC 9 escape or plain BEL) when a turn **completes successfully** and took longer than a threshold, so you can tab away while a long task runs. Failed or cancelled turns are intentionally silent — the notification is a "your task is ready" cue, not a generic ping. Configuration lives under `[notifications]`: ```toml [notifications]