Merge pull request #591 from Hmbown/fix/583-windows-bel-default-off

fix(notifications): default Windows Auto fallback to Off, not BEL (#583)

config.example.toml

@@ -281,11 +281,18 @@ 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, 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;<msg>\x07 (iTerm2-style; shows macOS notification)
 #                 bel:  plain \x07 beep
 #                 off:  disable entirely

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.

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;<msg>\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,
     }
 }
@@ -105,9 +122,13 @@ pub fn notify_done_to<W: Write>(
 
 /// 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,
@@ -274,7 +295,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 +312,52 @@ 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.
+    #[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);
+    }
+
+    /// #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");

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.
@@ -400,6 +414,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 **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]
+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;<msg>\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: