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: 2 additions & 2 deletions docs/specs/alert.md
Original file line number Diff line number Diff line change
Expand Up @@ -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. |

Expand Down Expand Up @@ -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`.
Expand Down
2 changes: 1 addition & 1 deletion docs/specs/alert.rationale.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
3 changes: 2 additions & 1 deletion lib/src/components/SettingsDialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,8 @@ export function SettingsDialog({ onClose }: { onClose: () => void }) {
/>
<div className={`${UNDER_SWITCH_INDENT} mt-1 text-sm leading-relaxed text-muted`}>
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.
</div>
</div>
</section>
Expand Down
16 changes: 16 additions & 0 deletions lib/src/lib/alert-manager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand Down
6 changes: 4 additions & 2 deletions lib/src/lib/alert-manager.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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<string>();
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 ---

Expand Down
10 changes: 7 additions & 3 deletions lib/src/lib/alert-settings-host.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 }),
);
});
});
2 changes: 1 addition & 1 deletion lib/src/lib/alert-settings-model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
7 changes: 6 additions & 1 deletion lib/src/lib/alert-settings.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
Expand All @@ -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);
});
});

Expand Down
8 changes: 4 additions & 4 deletions lib/src/stories/SettingsDialog.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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',
});
},
};
Expand Down
4 changes: 3 additions & 1 deletion standalone/src/browser-sidecar-adapter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
Expand Down
Loading