From 37ae7f20b8ac3d78c2c7ac38aea325db5d759e20 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Sat, 19 Sep 2026 23:34:45 -0700 Subject: [PATCH 1/2] Defer terminal-notification alerts until animation stops by default Coding agents emit their "done" notification while their TUI is still redrawing, so the undeferred ring summoned the user to a pane that was still animating. The switch stays as an escape hatch for the protocols' literal timing, and the AlertManager field now follows DEFAULT_ALERT_SETTINGS so a manager that never receives a settings blob behaves like one that does. Co-Authored-By: Claude Fable 5.1 --- docs/specs/alert.md | 2 +- docs/specs/alert.rationale.md | 2 +- lib/src/lib/alert-manager.test.ts | 16 ++++++++++++++++ lib/src/lib/alert-manager.ts | 6 ++++-- lib/src/lib/alert-settings-host.test.ts | 10 +++++++--- lib/src/lib/alert-settings-model.ts | 2 +- lib/src/lib/alert-settings.test.ts | 7 ++++++- lib/src/stories/SettingsDialog.stories.tsx | 8 ++++---- 8 files changed, 40 insertions(+), 13 deletions(-) diff --git a/docs/specs/alert.md b/docs/specs/alert.md index 7c262f1ed..3a5176df7 100644 --- a/docs/specs/alert.md +++ b/docs/specs/alert.md @@ -261,7 +261,7 @@ Application alarm defaults live beside the WATCHING rule set, edited in **Settin | Field | Meaning | |---|---| | `inactivityTimeoutMs` | `T_USER_ATTENTION` — the walk-away window defined under Attention. | -| `deferAlertsUntilQuiet` | Defer eligible terminal-notification rings while the animation watcher is fully armed. Default off. (rationale) | +| `deferAlertsUntilQuiet` | Defer eligible terminal-notification rings while the animation watcher is fully armed. Default on. (rationale) | | `speakEnabled` / `speakDelayMs` | Spoken alarms, below. | | `pushEnabled` / `pushDelayMs` | Push notifications, below. | diff --git a/docs/specs/alert.rationale.md b/docs/specs/alert.rationale.md index 0fcf750fd..c10711877 100644 --- a/docs/specs/alert.rationale.md +++ b/docs/specs/alert.rationale.md @@ -62,7 +62,7 @@ ## Alarm settings -**Why animation deferral defaults off.** BEL and notification OSCs explicitly ask to alert now, while continuously changing output may never become quiet. Opt-in preserves their established timing and makes indefinite deferral a deliberate choice. +**Why animation deferral defaults on.** Coding agents (`claude`, `codex`) send their notification OSC while their TUI is still redrawing its spinner, so an undeferred ring summons the user to a pane that is still animating (2026-09). The gate engages only while the private detector is fully armed, so a BEL from an otherwise quiet shell still rings at once. Turning the switch off restores the protocols' literal timing, including a ring that continuous output can never quiet. **Why the settings ride the WATCHING rule set's seed/broadcast shape.** Each VS Code webview has its own origin and therefore its own `localStorage`, while the `AlertManager` is shared; without a host-authoritative copy, two webviews would each believe their own blob. The one difference is the whole-blob relay: an alarm setting is not a set of independent keys the way a rule list is. diff --git a/lib/src/lib/alert-manager.test.ts b/lib/src/lib/alert-manager.test.ts index 393a4657e..8367e873d 100644 --- a/lib/src/lib/alert-manager.test.ts +++ b/lib/src/lib/alert-manager.test.ts @@ -102,6 +102,9 @@ describe('AlertManager in isolation', () => { it('ALERT_RINGING latches when user has no attention (view hidden)', () => { const id = 'latch-test'; + // Deferral ships on and withdraws a WATCHING ring once output resumes + // confirmed BUSY; latching through output is the switched-off timing. + manager.setDeferAlertsUntilQuiet(false); runWatchedCommand(id); manager.clearAttention(id); @@ -934,6 +937,17 @@ describe('AlertManager in isolation', () => { }); }); + it('defers a protocol alert with no settings call, because deferral ships on', () => { + const id = 'defer-shipped-default'; + driveToBusy(id); + + manager.notifyFromProtocol(id, { source: 'OSC 9', title: null, body: 'Done' }); + expect(manager.getState(id)).toMatchObject({ todo: false, notification: null }); + + vi.advanceTimersByTime(5_000); + expect(manager.getState(id)).toMatchObject({ status: 'ALERT_RINGING', todo: true }); + }); + describe('defer terminal notifications until quiet', () => { beforeEach(() => { manager.setDeferAlertsUntilQuiet(true); @@ -1489,6 +1503,8 @@ describe('AlertManager in isolation', () => { ['after the detector has noticed the output', 800], ] as const)('leaves a stale WATCHING ring alone once output has resumed, %s', async (_label, gapMs) => { const id = `await-stale-watching-ring-${gapMs}`; + // Keep the latched ring across resumed output: deferral would withdraw it. + manager.setDeferAlertsUntilQuiet(false); driveToRinging(id); // The peer was sent another turn and is talking again. Nothing clears the diff --git a/lib/src/lib/alert-manager.ts b/lib/src/lib/alert-manager.ts index 4c9769aa9..c41a9dacf 100644 --- a/lib/src/lib/alert-manager.ts +++ b/lib/src/lib/alert-manager.ts @@ -1,7 +1,7 @@ import { createAlertEpisode, type AlertEpisode } from './alert-episode'; import { QuiesceDetector, type QuiesceStatus, type QuiesceSnapshot } from './quiesce-detector'; import { applyTerminalProtocolEvents, collectTerminalSemanticEvents, type TerminalProtocolParseResult } from './terminal-protocol'; -import type { AlertSettings } from './alert-settings'; +import { DEFAULT_ALERT_SETTINGS, type AlertSettings } from './alert-settings-model'; import { cfg } from '../cfg'; import { commandArgv0, @@ -239,7 +239,9 @@ export class AlertManager { * drops them here, so a host marks the id once instead of guarding each call. */ private helpers = new Set(); private inactivityTimeoutMs = cfg.alert.userAttention; - private deferAlertsUntilQuiet = false; + /** The shipped default (platform-free module: this runs in both hosts), so a + * manager that never receives a settings blob behaves like one that does. */ + private deferAlertsUntilQuiet = DEFAULT_ALERT_SETTINGS.deferAlertsUntilQuiet; // --- Settings --- diff --git a/lib/src/lib/alert-settings-host.test.ts b/lib/src/lib/alert-settings-host.test.ts index ab8b7b0f3..125247366 100644 --- a/lib/src/lib/alert-settings-host.test.ts +++ b/lib/src/lib/alert-settings-host.test.ts @@ -28,14 +28,18 @@ describe('AlertSettingsHost', () => { it('keeps the first startup seed but always applies an explicit update', () => { const { host, target } = createHost(); - host.initialize({ deferAlertsUntilQuiet: true }); + // The seeded value is the non-default one, so a second seed winning would show. host.initialize({ deferAlertsUntilQuiet: false }); + host.initialize({ deferAlertsUntilQuiet: true }); expect(target.applySettings).toHaveBeenCalledTimes(1); + expect(target.applySettings).toHaveBeenCalledWith( + expect.objectContaining({ deferAlertsUntilQuiet: false }), + ); - host.update({ deferAlertsUntilQuiet: false }); + host.update({ deferAlertsUntilQuiet: true }); expect(target.applySettings).toHaveBeenNthCalledWith( 2, - expect.objectContaining({ deferAlertsUntilQuiet: false }), + expect.objectContaining({ deferAlertsUntilQuiet: true }), ); }); }); diff --git a/lib/src/lib/alert-settings-model.ts b/lib/src/lib/alert-settings-model.ts index 310349efb..31fdd587a 100644 --- a/lib/src/lib/alert-settings-model.ts +++ b/lib/src/lib/alert-settings-model.ts @@ -32,7 +32,7 @@ export const MAX_DELAY_MS = 600_000; export const DEFAULT_ALERT_SETTINGS: AlertSettings = { // cfg.ts stays the single source of the shipped default. inactivityTimeoutMs: cfg.alert.userAttention, - deferAlertsUntilQuiet: false, + deferAlertsUntilQuiet: true, speakEnabled: false, speakDelayMs: 10_000, pushEnabled: false, diff --git a/lib/src/lib/alert-settings.test.ts b/lib/src/lib/alert-settings.test.ts index 72a6d4fad..930b95fd4 100644 --- a/lib/src/lib/alert-settings.test.ts +++ b/lib/src/lib/alert-settings.test.ts @@ -50,6 +50,10 @@ describe('normalizeAlertSettings', () => { expect(DEFAULT_ALERT_SETTINGS.inactivityTimeoutMs).toBe(cfg.alert.userAttention); }); + it('ships animation deferral on', () => { + expect(normalizeAlertSettings({}).deferAlertsUntilQuiet).toBe(true); + }); + it('fills in missing keys and drops unknown ones', () => { const result = normalizeAlertSettings({ speakEnabled: true, bogus: 'x' }); expect(result).toEqual({ ...DEFAULT_ALERT_SETTINGS, speakEnabled: true }); @@ -76,7 +80,8 @@ describe('normalizeAlertSettings', () => { it('rejects non-boolean flags', () => { expect(normalizeAlertSettings({ speakEnabled: 'yes' }).speakEnabled).toBe(false); expect(normalizeAlertSettings({ speakEnabled: 1 }).speakEnabled).toBe(false); - expect(normalizeAlertSettings({ deferAlertsUntilQuiet: 'yes' }).deferAlertsUntilQuiet).toBe(false); + // Falsy non-booleans must not switch the on-by-default flag off either. + expect(normalizeAlertSettings({ deferAlertsUntilQuiet: 0 }).deferAlertsUntilQuiet).toBe(true); }); }); diff --git a/lib/src/stories/SettingsDialog.stories.tsx b/lib/src/stories/SettingsDialog.stories.tsx index 136261d3f..26aff9089 100644 --- a/lib/src/stories/SettingsDialog.stories.tsx +++ b/lib/src/stories/SettingsDialog.stories.tsx @@ -52,15 +52,15 @@ export const WithRules: Story = { }, }; -/** The animation watcher gates terminal-notification alerts. */ -export const DeferralEnabled: Story = { +/** The escape hatch: deferral off, so terminal notifications ring during animation. */ +export const DeferralDisabled: Story = { parameters: { primedWatchedCommands: ['claude', 'codex'], - primedAlertSettings: { deferAlertsUntilQuiet: true }, + primedAlertSettings: { deferAlertsUntilQuiet: false }, }, play: async ({ canvasElement }) => { await dialog(canvasElement).findByRole('switch', { - name: 'Defer alerts until animation stops on', + name: 'Defer alerts until animation stops off', }); }, }; From 8fe508f8ea8d7eb40baf035d806a56dc3c09d978 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Sun, 20 Sep 2026 01:58:52 -0700 Subject: [PATCH 2/2] Say what the deferral switch gates, now that it ships on The switch also gates withdrawal of a WATCHING ring when watched work resumes, so the default flip ships that out of the box too, while the helper text under the switch and the Alarm settings row described only the deferral half. Both now name both behaviors, and the settings row points at the section that owns each rule instead of restating one. The rationale leads with deferral being the unbounded state rather than reading as though the off position were, and records why an install that already saved a settings blob keeps the old value: with no version field to key a migration on, dropping a persisted false on read would leave the off position unpersistable. The browser-sidecar adapter's canonical blob differs from DEFAULT_ALERT_SETTINGS again, so the assertion still tells a forwarded blob apart from an emitted default. Co-Authored-By: Claude Opus 5 (1M context) --- docs/specs/alert.md | 4 ++-- docs/specs/alert.rationale.md | 2 +- lib/src/components/SettingsDialog.tsx | 3 ++- standalone/src/browser-sidecar-adapter.test.ts | 4 +++- 4 files changed, 8 insertions(+), 5 deletions(-) diff --git a/docs/specs/alert.md b/docs/specs/alert.md index 3a5176df7..75c2d773a 100644 --- a/docs/specs/alert.md +++ b/docs/specs/alert.md @@ -261,7 +261,7 @@ Application alarm defaults live beside the WATCHING rule set, edited in **Settin | Field | Meaning | |---|---| | `inactivityTimeoutMs` | `T_USER_ATTENTION` — the walk-away window defined under Attention. | -| `deferAlertsUntilQuiet` | Defer eligible terminal-notification rings while the animation watcher is fully armed. Default on. (rationale) | +| `deferAlertsUntilQuiet` | Gates animation deferral (Completion events) and resumed-ring withdrawal (WATCHING Track). Default on. (rationale) | | `speakEnabled` / `speakDelayMs` | Spoken alarms, below. | | `pushEnabled` / `pushDelayMs` | Push notifications, below. | @@ -330,7 +330,7 @@ Reached from the baseboard sliders; `docs/specs/layout.md` owns placement. The a - **Must toggle only the clicked baseboard alarm setting**, as an override for that Workspace, showing the effective value. Components without a Workspace scope edit application defaults. **Must show its shared settings section for 2 seconds, then fade for 250ms**, anchored to the button and bounded by the viewport. The preview is inert, announces the resulting state, preserves keyboard focus and command dispatch, and omits test actions. Each click replaces the preview and restarts its lifetime; opening Settings or unmounting clears it. Reduced motion skips the fade. Pinned by `Baseboard.test.tsx`. - Lists every watched command with a remove control, and **cannot add one** — WATCHING is keyed on a running command's name, so creating a rule stays a bell click / `a` press in the tab running it, and the empty state says so. With the bell dialog it is one of the two places a rule set on a since-closed Pane can be removed; both render the same `WatchedCommandList`. -- The watcher group carries the **Defer alerts until animation stops** switch and explains that only a fully armed watcher delays terminal notifications. +- The watcher group carries the **Defer alerts until animation stops** switch and explains that a fully armed watcher delays terminal notifications and withdraws a ring once watched work resumes. - **Delays are committed on blur or `Enter`, never per keystroke** — typing `3` on the way to `30` must not briefly install a 3-second timer. They are shown in seconds; an out-of-range or empty entry snaps back to whatever the store clamped it to. - **The push group's device line names every device a push would reach**, and otherwise says why there is none — no Burrow enrolled, nothing subscribed yet, or the server could not be asked (rationale). - **Must separate application defaults from this Workspace’s overrides** and offer per-field inheritance plus reset-all. The local voice picker follows engine voice availability. Pinned by `lib/src/components/WorkspaceAlarmSettings.test.tsx`. diff --git a/docs/specs/alert.rationale.md b/docs/specs/alert.rationale.md index c10711877..816202c5a 100644 --- a/docs/specs/alert.rationale.md +++ b/docs/specs/alert.rationale.md @@ -62,7 +62,7 @@ ## Alarm settings -**Why animation deferral defaults on.** Coding agents (`claude`, `codex`) send their notification OSC while their TUI is still redrawing its spinner, so an undeferred ring summons the user to a pane that is still animating (2026-09). The gate engages only while the private detector is fully armed, so a BEL from an otherwise quiet shell still rings at once. Turning the switch off restores the protocols' literal timing, including a ring that continuous output can never quiet. +**Why animation deferral defaults on.** Coding agents (`claude`, `codex`) send their notification OSC while their TUI is still redrawing its spinner, so an undeferred ring summons the user to a pane that is still animating (2026-09). The gate engages only while the private detector is fully armed, so a BEL from an otherwise quiet shell still rings at once. Deferral is unbounded, so continuous output can hold a ring indefinitely; turning the switch off is the escape hatch that restores the protocols' literal timing. Installs that saved any settings blob keep the old value: the blob has no version field, and a persisted `false` cannot be told from a deliberate opt-out, so dropping it on read would leave the off position unpersistable. **Why the settings ride the WATCHING rule set's seed/broadcast shape.** Each VS Code webview has its own origin and therefore its own `localStorage`, while the `AlertManager` is shared; without a host-authoritative copy, two webviews would each believe their own blob. The one difference is the whole-blob relay: an alarm setting is not a set of independent keys the way a rule list is. diff --git a/lib/src/components/SettingsDialog.tsx b/lib/src/components/SettingsDialog.tsx index 0d36db07d..ea5b4e3af 100644 --- a/lib/src/components/SettingsDialog.tsx +++ b/lib/src/components/SettingsDialog.tsx @@ -189,7 +189,8 @@ export function SettingsDialog({ onClose }: { onClose: () => void }) { />
When the animation watcher is fully armed, terminal notifications wait - for the pane to become quiet. + for the pane to become quiet, and a ring raised by silence goes away if + the watched command starts working again.
diff --git a/standalone/src/browser-sidecar-adapter.test.ts b/standalone/src/browser-sidecar-adapter.test.ts index c1069d6c6..e38e971a0 100644 --- a/standalone/src/browser-sidecar-adapter.test.ts +++ b/standalone/src/browser-sidecar-adapter.test.ts @@ -191,7 +191,9 @@ describe("BrowserSidecarAdapter terminal stream", () => { expect(setWatched).toHaveBeenCalledWith(["cargo", "make"]); expect(names).toEqual([["cargo", "make"]]); - const canonical: AlertSettings = { ...DEFAULT_ALERT_SETTINGS, deferAlertsUntilQuiet: true }; + // Not the default blob, so the assertion still distinguishes "forwarded what + // it was handed" from "emitted DEFAULT_ALERT_SETTINGS". + const canonical: AlertSettings = { ...DEFAULT_ALERT_SETTINGS, deferAlertsUntilQuiet: false }; deliver("alert:settings", { settings: canonical }); expect(applySettings).toHaveBeenCalledWith(canonical); expect(settings).toEqual([canonical]);