diff --git a/docs/specs/alert.md b/docs/specs/alert.md index 7c262f1ed..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 off. (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 0fcf750fd..816202c5a 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. 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/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', }); }, }; 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]);