From 04224dcd5b2f397599ad7da3ebdae2d0807b06a9 Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Sat, 19 Sep 2026 23:49:03 -0700 Subject: [PATCH 1/2] Make the alert action dismiss and open the context, never edit a rule MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The pane-header alert button, `a` in command mode, and the mobile header button ran a five-way transition table that could silently turn a WATCHING rule on or off. They now do one thing: dismiss a ringing Session (setting `todo`, as before) and open the terminal context. WATCHING rules are created and removed in the terminal context, and removed for a since-closed Pane in Settings — the only place that can reach one. `attentionDismissedRing` existed only to stop the click after an attention-based dismissal from editing a rule, so it is gone from `AlertState`, the manager entry, `alertStatesEqual`, and the side map that kept it off the public Activity snapshot; `ActivityState` is now plain `AlertState`. No wire or persisted type carried it. `dismissOrToggleAlert`, `AlertButtonActionResult`, `toggleSessionAlert`, and `disableSessionAlert` are deleted with it. Co-Authored-By: Claude Fable 5.1 --- docs/specs/alert.md | 29 ++--- docs/specs/shortcuts.md | 2 +- lib/src/components/MobileWall.test.tsx | 2 +- lib/src/components/MobileWall.tsx | 24 ++-- lib/src/components/SettingsDialog.tsx | 10 +- lib/src/components/Wall.tsx | 7 +- lib/src/components/WatchedCommandList.tsx | 7 +- .../wall/TerminalPaneHeader.test.tsx | 2 +- .../components/wall/TerminalPaneHeader.tsx | 40 +++---- .../keyboard/handle-pane-shortcuts.test.ts | 6 +- .../wall/keyboard/handle-pane-shortcuts.ts | 46 ++++---- lib/src/components/wall/wall-context.tsx | 8 +- lib/src/components/wall/wall-test-utils.ts | 2 +- lib/src/lib/alert-manager.test.ts | 45 +++----- lib/src/lib/alert-manager.ts | 29 ++--- lib/src/lib/session-activity-store.ts | 81 ++----------- lib/src/lib/terminal-registry.alert.test.ts | 106 ++++++------------ lib/src/lib/terminal-registry.ts | 5 +- .../stories/BrowserChromeHeader.stories.tsx | 2 +- lib/src/stories/MouseHeaderIcon.stories.tsx | 2 +- lib/src/stories/ShellCwd.stories.tsx | 2 +- .../stories/TerminalPaneHeader.stories.tsx | 7 +- scripts/spec-word-budgets.json | 2 +- vscode-ext/README.md | 2 +- vscode-ext/test/session-state.test.ts | 1 - .../lib/__snapshots__/tut-runner.test.ts.snap | 6 +- website/src/lib/tut-items.ts | 6 +- website/src/pages/PlaygroundDesktop.tsx | 4 +- 28 files changed, 164 insertions(+), 321 deletions(-) diff --git a/docs/specs/alert.md b/docs/specs/alert.md index 3a5176df7..e9cb4a100 100644 --- a/docs/specs/alert.md +++ b/docs/specs/alert.md @@ -107,7 +107,7 @@ An **await** parks on one Session until it finishes what it is doing, then repor - **Skip a command-exit ring while a foreground command is running** (rationale). - **Skip a WATCHING ring once output has resumed since it latched** (`outputSinceWatchingRing`), and **never stand the detector in for that flag** (rationale). - **Never skip the bell**: an `OSC 9` is a discrete "I need input" that stays true until it is answered. -- **Consuming releases that one track's latch and nothing else** — `todo` is neither set nor cleared, no `ActivityNotification` is dropped, `attentionDismissedRing` is untouched, and `attentionId` is never set. +- **Consuming releases that one track's latch and nothing else** — `todo` is neither set nor cleared, no `ActivityNotification` is dropped, and `attentionId` is never set. **Absorption: absorb the summons, keep the receipt.** @@ -233,16 +233,14 @@ Source of truth: `applyCommandExitRinging` / `formatCommandExitBody` in `lib/src Clearing behavior: -- Attending a ringing Session clears active rings on all three tracks, sets `todo = true`, and sets `attentionDismissedRing = true`. -- Dismissing the ring from the bell or `a` (Pane Header) sets `todo = true` and opens the alert/TODO dialog. +- Attending a ringing Session clears active rings on all three tracks and sets `todo = true`. +- Dismissing the ring from the alert button or `a` (Pane Header) sets `todo = true` and opens the terminal context. **Dismissing a Session with nothing ringing changes nothing and notifies no one.** - Marking TODO clears any active ring and leaves the WATCHING rule in place for future cycles. - **Must clear notification and active rings when clearing TODO, even if `todo` is already false.** Pinned by `clears a WATCHING ring before it has created a TODO` in `lib/src/lib/alert-manager.test.ts`. - Passthrough `Enter` typed into the Session clears TODO. Command-mode `Enter` that only enters passthrough does not. - Removing a WATCHING rule turns watching off wherever it matched and silences the WATCHING rings it raised. It does not stop the detector, nor clear protocol progress, command-exit arms, TODO, or notification detail. - Destroying the Session clears all alert, TODO, notification, attention, protocol, and command-exit state. -`attentionDismissedRing` exists so the next bell click after an attention-based dismissal opens the dialog instead of silently editing a rule. **Only the explicit dismiss path consumes the flag** — turning WATCHING on or off, or advancing another alarm track, does not. - ## Live Workspace transfer - **Must transfer live alert state separately from persisted reminders:** track latches, episode, command watch, deferred notification, and detector history/deadlines travel in the marked transfer content. **Never read this content on cold restore.** @@ -329,7 +327,7 @@ Source of truth: `watchPushRings` / `invalidatePushDeviceRefreshes` in `lib/src/ Reached from the baseboard sliders; `docs/specs/layout.md` owns placement. The alarm sections sit under the theme and shell rows; when both are hidden (VS Code owns the theme and the shells), the rule list is first and drops its section divider. - **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`. +- 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 the terminal context of a Pane running it, and the empty state says so. **It is the only place a rule set on a since-closed Pane can be removed**, the terminal context reaching only the command its own Pane is running. - The watcher group carries the **Defer alerts until animation stops** switch and explains that only a fully armed watcher delays terminal notifications. - **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). @@ -372,19 +370,14 @@ Where it surfaces is host-specific: ### Pane Header -The header shows an alert bell, a fixed-text `TODO` pill when `todo === true`, a hover/focus notification preview when TODO has `notification`, and the terminal context opened by right-click or by some left-click actions. Placement, sizing, and width tiers belong to `docs/specs/layout.md`. - -Bell rotation follows public status; motion follows latch edges. **When a track latches, ring each mounted bell for four 800ms cycles, then hold 45° until the ring clears** (test: `runs a finite ringing burst and then holds the bell at 45 degrees` in `lib/src/components/bell-icon-class.test.ts`; rationale). **A newly mounted ringing bell may replay once without advancing `ringSeq`** (test: `replays the finite burst when a ringing presentation remounts` in `lib/src/components/AlertBell.test.tsx`; rationale). **A newly latched track replays the burst; further reports on that track only enrich its summons.** `AlertState.ringSeq` counts per-Session latches and is compared by `alertStatesEqual` (tests: `counts a second track ringing behind an already-latched one` and `does not count a track that is already ringing` in `lib/src/lib/alert-manager.test.ts`, `replaces the icon when the ring counter advances` in `lib/src/components/AlertBell.test.tsx`; rationale). **Remote Clients have no counter:** `DirectoryEntry.ringing` is an edgeless boolean, so Pocket rings on mount and holds. **The bell names the command it would act on** ("Alert on all `claude`"), not an abstract toggle — that is the scope of what a click changes. +The header shows an alert bell, a fixed-text `TODO` pill when `todo === true`, a hover/focus notification preview when TODO has `notification`, and the terminal context opened by right-click or by the alert button. Placement, sizing, and width tiers belong to `docs/specs/layout.md`. -Bell interactions — one transition table, in `dismissOrToggleAlert`: +Bell rotation follows public status; motion follows latch edges. **When a track latches, ring each mounted bell for four 800ms cycles, then hold 45° until the ring clears** (test: `runs a finite ringing burst and then holds the bell at 45 degrees` in `lib/src/components/bell-icon-class.test.ts`; rationale). **A newly mounted ringing bell may replay once without advancing `ringSeq`** (test: `replays the finite burst when a ringing presentation remounts` in `lib/src/components/AlertBell.test.tsx`; rationale). **A newly latched track replays the burst; further reports on that track only enrich its summons.** `AlertState.ringSeq` counts per-Session latches and is compared by `alertStatesEqual` (tests: `counts a second track ringing behind an already-latched one` and `does not count a track that is already ringing` in `lib/src/lib/alert-manager.test.ts`, `replaces the icon when the ring counter advances` in `lib/src/components/AlertBell.test.tsx`; rationale). **Remote Clients have no counter:** `DirectoryEntry.ringing` is an edgeless boolean, so Pocket rings on mount and holds. -- Left-click `ALERT_RINGING`: dismiss, create TODO if needed, open context. -- Left-click after `attentionDismissedRing`: consume the flag and open context. -- Otherwise, with a command running: toggle that command's WATCHING rule on or off. Turning it off drops the rule for every Session running it. -- Exception: from `OSC_NOTIF_BUSY` or `COMMAND_EXIT_ARMED` with no rule set, open the context instead. Those alarms need no rule, so a click must not create one by surprise, and must not clear the progress or the arm. -- With no command running: change nothing and open the context, which explains that alerts are per command. -- Pressing `a` on the selected Pane in command mode uses the same action. Right-click always opens the context. -- Pressing `t` toggles TODO. +- **The alert button, and `a` on the selected Pane in command mode, dismiss a ringing Session and open the terminal context, whatever the status; they never edit a WATCHING rule.** The button names that action — `Dismiss alert` while ringing, `Alert settings` otherwise — not a toggle. +- **A WATCHING rule is created only in the terminal context** ("Watch all `` commands"), which offers the row whenever a foreground command is running, and removed there or in Settings. Removing it anywhere drops it for every Session running that command. +- Right-click always opens the context. Pressing `t` toggles TODO. +- **The mobile header's alert button only dismisses** — the mobile composition has no terminal context (`docs/specs/mobile-terminal-ui.md`). **Must keep context alert controls scoped to the source**, with TODO, running-command WATCHING, and notification detail. Settings owns the global watched-command list. **Must suppress helper alerting until promotion, including after exit**, covering bell/notification protocols, watched commands, TODO, speech, push, and attention projections; semantic command/readiness state remains active. Promotion starts ordinary alert behavior without replaying suppressed events. @@ -394,7 +387,7 @@ The TODO pill always displays `TODO`; remote notification text belongs in previe Spoken-alarm delivery is much louder than the bell: a pointer-transparent treatment spans the whole terminal Pane, labelled `SPEAKING` while the engine actually speaks and `SPOKEN` — quieter, and unbounded — until the ring resolves. **`prefers-reduced-motion` keeps the strong static treatment and suppresses only the pulse**, as does `cfg.alert.ringingPaused` (rationale). The layers, their strengths, placement, and sizing belong to `docs/specs/layout.md` → Spoken-alarm overlay. -Source of truth: `AlertBell` in `lib/src/components/AlertBell.tsx`; `bellIconClass` in `lib/src/components/bell-icon-class.ts`; `latchRing` in `lib/src/lib/alert-manager.ts`; `dismissOrToggleAlert` in `lib/src/lib/session-activity-store.ts`; `lib/src/components/TodoPillBody.tsx`; `lib/src/components/wall/AlertSpeechIndicator.tsx`. +Source of truth: `AlertBell` in `lib/src/components/AlertBell.tsx`; `bellIconClass` in `lib/src/components/bell-icon-class.ts`; `latchRing` in `lib/src/lib/alert-manager.ts`; `dismissSessionAlert` in `lib/src/lib/session-activity-store.ts`; `TerminalContext` in `lib/src/components/wall/TerminalContext.tsx`; `lib/src/components/TodoPillBody.tsx`; `lib/src/components/wall/AlertSpeechIndicator.tsx`. ### Door diff --git a/docs/specs/shortcuts.md b/docs/specs/shortcuts.md index 58e868ac3..4fcdd5ece 100644 --- a/docs/specs/shortcuts.md +++ b/docs/specs/shortcuts.md @@ -24,7 +24,7 @@ A focused cross-origin iframe surface swallows the gesture; the proxy shim detec | `m` or `d` | Minimize / reattach | Stays in command mode, unlike `Enter` on a door. | | `k` or `x` | Kill | Kills the selected pane or door behind a random-letter prompt; an untouched Surface skips it. | | `,` | Rename | Inline rename of the selected terminal pane's title; consumed no-op on browser surfaces and doors. | -| `a` | Toggle alert | Dismiss or toggle the bell alert. Terminal Surfaces only; doors excluded. | +| `a` | Alert | Dismiss the ring if any, then open the terminal context. Terminal Surfaces only; doors excluded. | | `t` | Toggle todo | Toggle the TODO marker on the selected Surface, terminal or browser; doors excluded. | | `>` | Terminal context | Terminal panes only; consumed no-op on browser panes, inert on doors. | diff --git a/lib/src/components/MobileWall.test.tsx b/lib/src/components/MobileWall.test.tsx index 1b7f7f2d2..d86f53022 100644 --- a/lib/src/components/MobileWall.test.tsx +++ b/lib/src/components/MobileWall.test.tsx @@ -11,7 +11,7 @@ globalThis.IS_REACT_ACT_ENVIRONMENT = true; const registry = vi.hoisted(() => ({ activitySnapshot: new Map(), clearSessionTodo: vi.fn(), - dismissOrToggleAlert: vi.fn(), + dismissSessionAlert: vi.fn(), disposeSession: vi.fn(), getActivitySnapshot: vi.fn(), getOrCreateTerminal: vi.fn(), diff --git a/lib/src/components/MobileWall.tsx b/lib/src/components/MobileWall.tsx index c131f3267..351ac314a 100644 --- a/lib/src/components/MobileWall.tsx +++ b/lib/src/components/MobileWall.tsx @@ -12,7 +12,7 @@ import type { MobileTerminalSessionItem } from './MobileTerminalUi'; import { clearSessionTodo, DEFAULT_ACTIVITY_STATE, - dismissOrToggleAlert, + dismissSessionAlert, disposeSession, getActivitySnapshot, getOrCreateTerminal, @@ -48,16 +48,10 @@ export interface MobileWallProps { const DEFAULT_MOBILE_SESSION: MobileWallSession = { id: 'mobile-pane' }; -const ALERT_BUTTON_LABELS: Record = { - WATCHING_DISABLED: { aria: 'Enable watching', tooltip: 'Enable watching' }, - NOTHING_TO_SHOW: { aria: 'Disable watching', tooltip: 'Disable watching' }, - MIGHT_BE_BUSY: { aria: 'Disable watching', tooltip: 'Disable watching' }, - BUSY: { aria: 'Disable watching', tooltip: 'Disable watching' }, - MIGHT_NEED_ATTENTION: { aria: 'Disable watching', tooltip: 'Disable watching' }, - ALERT_RINGING: { aria: 'Alert ringing', tooltip: 'Alert ringing' }, - OSC_NOTIF_BUSY: { aria: 'Progress active', tooltip: 'Progress active' }, - COMMAND_EXIT_ARMED: { aria: 'Command running', tooltip: 'Command running' }, -}; +// Mobile has no terminal context, so dismissing a ring is the button's whole +// action; it never edits a rule (`docs/specs/alert.md` -> Pane Header). +const alertButtonLabelFor = (status: SessionStatus): string => + status === 'ALERT_RINGING' ? 'Dismiss alert' : 'Alert status'; export function useMobileWallSessionItems( sessions: MobileWallSession[], @@ -185,7 +179,7 @@ function MobileWallHeader({ }) { const status = session.status ?? 'WATCHING_DISABLED'; const todoPill = useTodoPillContent(session.todo === true); - const alertButtonLabels = ALERT_BUTTON_LABELS[status]; + const alertButtonLabel = alertButtonLabelFor(status); const showTodoPill = todoPill.visible; return ( @@ -197,9 +191,9 @@ function MobileWallHeader({ 'flex h-5 min-w-5 items-center justify-center rounded transition-colors shrink-0 hover:bg-current/10', status === 'ALERT_RINGING' ? 'text-alarm-vs-header-active' : '', ].join(' ')} - onClick={() => dismissOrToggleAlert(session.id, status)} - ariaLabel={alertButtonLabels.aria} - tooltip={alertButtonLabels.tooltip} + onClick={() => dismissSessionAlert(session.id)} + ariaLabel={alertButtonLabel} + tooltip={alertButtonLabel} tooltipAlign="left" dataAlertButtonFor={session.id} > diff --git a/lib/src/components/SettingsDialog.tsx b/lib/src/components/SettingsDialog.tsx index 0d36db07d..cd83ca2fe 100644 --- a/lib/src/components/SettingsDialog.tsx +++ b/lib/src/components/SettingsDialog.tsx @@ -84,9 +84,9 @@ function describePushTargets(push: PushDevicesState, remoteControlBelow: boolean * (`docs/specs/alert.md` -> Alarm settings). * * Rules are removable here but not addable: WATCHING is keyed on a running - * command's name, so a rule is created by pressing `a` in the tab running it. - * This dialog and the bell popover are the two places a rule set on a - * since-closed Pane can be found and removed. + * command's name, so a rule is created in the terminal context of a Pane + * running it. This dialog is the one place a rule set on a since-closed Pane + * can be found and removed. */ export function SettingsDialog({ onClose }: { onClose: () => void }) { const watched = useSyncExternalStore(subscribeToWatchedCommands, getWatchedCommandsSnapshot); @@ -177,8 +177,8 @@ export function SettingsDialog({ onClose }: { onClose: () => void }) { ) : (
- Nothing yet. Start a command, then press a in its tab to - alert on every tab running it. + Nothing yet. Start a command, then press a in its tab and + turn on Watch all … to alert on every tab running it.
)}
diff --git a/lib/src/components/Wall.tsx b/lib/src/components/Wall.tsx index 1aebb8424..de0f65355 100644 --- a/lib/src/components/Wall.tsx +++ b/lib/src/components/Wall.tsx @@ -38,7 +38,7 @@ import { clearLocalSurfaceActivity, deriveSessionLabel, disposeSession, - dismissOrToggleAlert, + dismissSessionAlert, focusSession, refitSession, markSessionAttention, @@ -54,7 +54,6 @@ import { countRunningSessionsIn, setTerminalUserTitle, UNNAMED_PANEL_TITLE, - type SessionStatus, } from '../lib/terminal-registry'; import { buildAppTitleResolver, @@ -1895,8 +1894,8 @@ export function Wall({ exitTerminalMode(); requestKill(id); }, - onAlertButton: (id: string, displayedStatus: SessionStatus) => { - return dismissOrToggleAlert(id, displayedStatus); + onAlertButton: (id: string) => { + dismissSessionAlert(id); }, onToggleTodo: (id: string) => { toggleSessionTodo(id); diff --git a/lib/src/components/WatchedCommandList.tsx b/lib/src/components/WatchedCommandList.tsx index 11023ed4f..1a3e2f5e3 100644 --- a/lib/src/components/WatchedCommandList.tsx +++ b/lib/src/components/WatchedCommandList.tsx @@ -11,12 +11,11 @@ import { * The app-global WATCHING rule set, with a remove control per rule * (`docs/specs/alert.md` -> WATCHING Track). * - * Rendered by both the bell popover and the Alarm settings dialog — the two - * places a rule set on a since-closed Pane can be found and removed. It is one - * list shown twice, so it lives here rather than in either dialog. + * Rendered only by the Alarm settings dialog — the one place a rule set on a + * since-closed Pane can be found and removed. * * Rules are removable but not addable: WATCHING is keyed on a running command's - * name, so creating one stays a bell click in the tab running it. + * name, so creating one stays the terminal context of a Pane running it. */ export function WatchedCommandList() { const watched = useSyncExternalStore(subscribeToWatchedCommands, getWatchedCommandsSnapshot); diff --git a/lib/src/components/wall/TerminalPaneHeader.test.tsx b/lib/src/components/wall/TerminalPaneHeader.test.tsx index a0617ec3a..bd8cfecee 100644 --- a/lib/src/components/wall/TerminalPaneHeader.test.tsx +++ b/lib/src/components/wall/TerminalPaneHeader.test.tsx @@ -184,7 +184,7 @@ describe('TerminalPaneHeader — notepad icon', () => { const labels = Array.from(container.querySelectorAll('button[aria-label]')) .map((button) => button.getAttribute('aria-label')); expect(labels).toEqual([ - 'Alerts are per command', + 'Alert settings', 'Override mouse capture', 'Notepad', 'Split left/right', diff --git a/lib/src/components/wall/TerminalPaneHeader.tsx b/lib/src/components/wall/TerminalPaneHeader.tsx index 46d019c01..29c49044f 100644 --- a/lib/src/components/wall/TerminalPaneHeader.tsx +++ b/lib/src/components/wall/TerminalPaneHeader.tsx @@ -35,7 +35,6 @@ import { } from '../../lib/terminal-registry'; import { buildAppTitleResolver, - commandArgv0, createTerminalPaneState, COMMAND_FAIL_GLYPH, deriveHeader, @@ -80,16 +79,12 @@ const terminalHeaderTier = (width: number): TerminalHeaderTier => : width > 98 ? 'bare' : 'tiny'; -// WATCHING is a rule on the running command, so the bell says which command it -// would act on rather than naming an abstract toggle (`docs/specs/alert.md`). -function alertButtonLabelsFor(status: SessionStatus, argv0: string | null): { aria: string; tooltip: string } { - if (status === 'ALERT_RINGING') return { aria: 'Alert ringing', tooltip: 'Alert ringing' }; - if (status === 'OSC_NOTIF_BUSY') return { aria: 'Progress active', tooltip: 'Progress active' }; - if (status === 'COMMAND_EXIT_ARMED') return { aria: 'Command running', tooltip: 'Command running' }; - if (!argv0) return { aria: 'Alerts are per command', tooltip: '[a] Alerts are per command' }; - return status === 'WATCHING_DISABLED' - ? { aria: `Alert on all ${argv0}`, tooltip: `[a] Alert on all "${argv0}"` } - : { aria: `Stop alerting on all ${argv0}`, tooltip: `[a] Stop alerting on all "${argv0}"` }; +// The button dismisses and opens the terminal context; it never edits a rule, +// so it names the action, not a toggle (`docs/specs/alert.md` -> Pane Header). +function alertButtonLabelsFor(status: SessionStatus): { aria: string; tooltip: string } { + return status === 'ALERT_RINGING' + ? { aria: 'Alert ringing', tooltip: '[a] Dismiss alert' } + : { aria: 'Alert settings', tooltip: '[a] Alert settings' }; } const TODO_PREVIEW_GAP = 6; const TODO_PREVIEW_MARGIN = 8; @@ -152,15 +147,12 @@ export function TerminalPaneHeader({ id, title, params }: PaneProps) { const roomForNotepad = compactOrWider || tier === 'minimal' || (tier === 'minimal-tight' && !dirty); const tiny = tier === 'tiny'; const showTodoPill = todoPill.visible && compactOrWider; - const runningArgv0 = paneState.currentCommand?.rawCommandLine - ? commandArgv0(paneState.currentCommand.rawCommandLine) - : null; - const alertButtonLabels = alertButtonLabelsFor(activity.status, runningArgv0); + const alertButtonLabels = alertButtonLabelsFor(activity.status); const alertButtonAriaLabel = alertButtonLabels.aria; const alertButtonTooltip = alertButtonLabels.tooltip; const alertButtonTooltipDetail = activity.status === 'ALERT_RINGING' ? 'Click to dismiss and show options' - : 'Right-click for options'; + : 'Click or right-click for options'; const todoNotificationPreview = formatNotificationPreview(activity.notification); const todoPreviewId = `todo-notification-preview-${id}`; @@ -180,14 +172,10 @@ export function TerminalPaneHeader({ id, title, params }: PaneProps) { setTodoPreviewRect(button.getBoundingClientRect()); }, [activity.notification]); - const triggerAlertButtonAction = useCallback((displayedStatus: SessionStatus, button: HTMLButtonElement) => { - const result = actions.onAlertButton(id, displayedStatus); - // 'no-command' opens the dialog too — it is where we explain that alerts are - // keyed on the running command and there is nothing running here. - if (result === 'dismissed' || result === 'menu' || result === 'no-command') { - const rect = button.getBoundingClientRect(); - context.open(id, { origin: { x: rect.left + rect.width / 2, y: rect.top + rect.height / 2 } }); - } + const triggerAlertButtonAction = useCallback((button: HTMLButtonElement) => { + actions.onAlertButton(id); + const rect = button.getBoundingClientRect(); + context.open(id, { origin: { x: rect.left + rect.width / 2, y: rect.top + rect.height / 2 } }); }, [actions, id, context]); useEffect(() => { @@ -247,14 +235,14 @@ export function TerminalPaneHeader({ id, title, params }: PaneProps) { e.preventDefault(); e.stopPropagation(); e.nativeEvent.stopImmediatePropagation?.(); - triggerAlertButtonAction(activity.status, e.currentTarget); + triggerAlertButtonAction(e.currentTarget); }} onClick={(e) => { if (suppressAlertClickRef.current) { suppressAlertClickRef.current = false; return; } - triggerAlertButtonAction(activity.status, e.currentTarget); + triggerAlertButtonAction(e.currentTarget); }} onContextMenu={(e) => context.open(id, { origin: { x: e.clientX, y: e.clientY } })} ariaLabel={alertButtonAriaLabel} diff --git a/lib/src/components/wall/keyboard/handle-pane-shortcuts.test.ts b/lib/src/components/wall/keyboard/handle-pane-shortcuts.test.ts index aa65a9920..c290e794b 100644 --- a/lib/src/components/wall/keyboard/handle-pane-shortcuts.test.ts +++ b/lib/src/components/wall/keyboard/handle-pane-shortcuts.test.ts @@ -9,14 +9,12 @@ import { useWallKeyboard } from '../use-wall-keyboard'; import type { WallKeyboardCtx } from './types'; const terminalRegistryMocks = vi.hoisted(() => ({ - dismissOrToggleAlert: vi.fn(), - getActivity: vi.fn(() => ({ status: 'WATCHING_DISABLED' })), + dismissSessionAlert: vi.fn(), toggleSessionTodo: vi.fn(), })); vi.mock('../../../lib/terminal-registry', () => ({ - dismissOrToggleAlert: terminalRegistryMocks.dismissOrToggleAlert, - getActivity: terminalRegistryMocks.getActivity, + dismissSessionAlert: terminalRegistryMocks.dismissSessionAlert, toggleSessionTodo: terminalRegistryMocks.toggleSessionTodo, })); diff --git a/lib/src/components/wall/keyboard/handle-pane-shortcuts.ts b/lib/src/components/wall/keyboard/handle-pane-shortcuts.ts index f2cb50e99..d3f624c2f 100644 --- a/lib/src/components/wall/keyboard/handle-pane-shortcuts.ts +++ b/lib/src/components/wall/keyboard/handle-pane-shortcuts.ts @@ -1,8 +1,4 @@ -import { - dismissOrToggleAlert, - getActivity, - toggleSessionTodo, -} from '../../../lib/terminal-registry'; +import { dismissSessionAlert, toggleSessionTodo } from '../../../lib/terminal-registry'; import { hasTerminal } from 'dor/commands/types'; import { surfaceKindFromParams } from '../browser-surface'; import { isWorkspaceSelection } from '../wall-types'; @@ -16,6 +12,23 @@ function findPaneHeaderForSession(id: string): HTMLElement | null { return document.querySelector(`[data-pane-header-for="${CSS.escape(id)}"]`); } +/** Reuse the header's own onContextMenu path: dispatch a synthetic contextmenu + * at the header's bottom-left corner as the reveal origin. Browser-surface + * panes carry no `data-pane-header-for`, so the lookup misses and the caller's + * key is a consumed no-op — the spec'd behavior for surfaces with no header + * context menu. */ +function openHeaderContextMenu(id: string): void { + const header = findPaneHeaderForSession(id); + if (!header) return; + const rect = header.getBoundingClientRect(); + header.dispatchEvent(new MouseEvent('contextmenu', { + bubbles: true, + cancelable: true, + clientX: rect.left, + clientY: rect.bottom, + })); +} + /** Keep aligned with the pane handlers below. Workspace selections consume * these keys; an omitted key returns unhandled without dispatching a pane action. * Pinned by the independent key list in handle-pane-navigation.test.ts. */ @@ -132,9 +145,14 @@ export function handlePaneShortcuts( if (ctx.dialogKeyboardActiveRef.current) return true; e.preventDefault(); e.stopPropagation(); + // Click the mounted button so the context is anchored to it; without one + // (a hidden or bare header) dismiss directly and anchor at the header. const alertButton = findAlertButtonForSession(sid); if (alertButton) alertButton.click(); - else dismissOrToggleAlert(sid, getActivity(sid).status); + else { + dismissSessionAlert(sid); + openHeaderContextMenu(sid); + } return true; } @@ -148,21 +166,7 @@ export function handlePaneShortcuts( if (e.key === '>' && sid && ctx.selectedTypeRef.current === 'pane') { e.preventDefault(); e.stopPropagation(); - // Reuse the header's own onContextMenu path: dispatch a synthetic - // contextmenu at the header's bottom-left corner as the reveal origin. - // Browser-surface panes carry no - // `data-pane-header-for`, so the lookup misses and the key is a consumed - // no-op — the spec'd behavior for surfaces with no header context menu. - const header = findPaneHeaderForSession(sid); - if (header) { - const rect = header.getBoundingClientRect(); - header.dispatchEvent(new MouseEvent('contextmenu', { - bubbles: true, - cancelable: true, - clientX: rect.left, - clientY: rect.bottom, - })); - } + openHeaderContextMenu(sid); return true; } diff --git a/lib/src/components/wall/wall-context.tsx b/lib/src/components/wall/wall-context.tsx index cce080ae6..b35a60022 100644 --- a/lib/src/components/wall/wall-context.tsx +++ b/lib/src/components/wall/wall-context.tsx @@ -3,7 +3,7 @@ import type { PortMode } from './TerminalContextView'; import type { PortUrlEntry } from './port-url'; import { createContext, useContext, useEffect, type RefObject } from 'react'; import type { RingFrame } from '../../lib/rect-tween'; -import type { AlertButtonActionResult, SessionStatus, SetTerminalUserTitleResult } from '../../lib/terminal-registry'; +import type { SetTerminalUserTitleResult } from '../../lib/terminal-registry'; import type { WallMode } from './wall-types'; import type { RenderMode } from './agent-browser-screen'; @@ -41,7 +41,9 @@ export const DoorElementsContext = createContext({ export interface WallActions { onKill: (id: string) => void; onMinimize: (id: string) => void; - onAlertButton: (id: string, displayedStatus: SessionStatus) => AlertButtonActionResult; + /** Dismiss a ringing Session. The caller then opens the terminal context + * (`docs/specs/alert.md` -> Pane Header). */ + onAlertButton: (id: string) => void; onToggleTodo: (id: string) => void; onSplitH: (id: string | null, source?: 'keyboard' | 'mouse') => void; onSplitV: (id: string | null, source?: 'keyboard' | 'mouse') => void; @@ -74,7 +76,7 @@ export interface WallActions { export const WallActionsContext = createContext({ onKill: () => {}, onMinimize: () => {}, - onAlertButton: () => 'noop', + onAlertButton: () => {}, onToggleTodo: () => {}, onSplitH: () => {}, onSplitV: () => {}, diff --git a/lib/src/components/wall/wall-test-utils.ts b/lib/src/components/wall/wall-test-utils.ts index 42460ddfd..fba777da7 100644 --- a/lib/src/components/wall/wall-test-utils.ts +++ b/lib/src/components/wall/wall-test-utils.ts @@ -15,7 +15,7 @@ export function stubWallActions(overrides: Partial = {}): WallActio return { onKill: vi.fn(), onMinimize: vi.fn(), - onAlertButton: vi.fn(() => 'noop'), + onAlertButton: vi.fn(), onToggleTodo: vi.fn(), onSplitH: vi.fn(), onSplitV: vi.fn(), diff --git a/lib/src/lib/alert-manager.test.ts b/lib/src/lib/alert-manager.test.ts index 8367e873d..afe5f31a7 100644 --- a/lib/src/lib/alert-manager.test.ts +++ b/lib/src/lib/alert-manager.test.ts @@ -309,44 +309,30 @@ describe('AlertManager in isolation', () => { }); }); - it('attending a ring leaves attentionDismissedRing for the bell table to consume', () => { - const id = 'attention-dismissed-watching-disabled'; + it('attending a ring clears it and leaves the TODO behind', () => { + const id = 'attention-clears-ring'; - // A protocol ring needs no WATCHING; attending it dismisses the ring and - // sets attentionDismissedRing while status falls back to WATCHING_DISABLED. + // A protocol ring needs no WATCHING, so status falls back to + // WATCHING_DISABLED once attention clears the latch. manager.notifyFromProtocol(id, { source: 'OSC 9', title: null, body: 'Build finished' }); expect(manager.getState(id).status).toBe('ALERT_RINGING'); - manager.attend(id); - expect(manager.getState(id)).toMatchObject({ - status: 'WATCHING_DISABLED', - todo: true, - attentionDismissedRing: true, - }); - // An explicit dismiss is the click that consumes the flag. - manager.dismissAlert(id); - expect(manager.getState(id).attentionDismissedRing).toBe(false); + manager.attend(id); + expect(manager.getState(id)).toMatchObject({ status: 'WATCHING_DISABLED', todo: true }); }); - it('keeps attentionDismissedRing when a watched command starts before bell dismissal', () => { - const id = 'attention-dismissed-then-watched-command'; - manager.setWatchedCommands(['claude']); + it('dismissing a Session with nothing ringing changes nothing and notifies no one', () => { + const id = 'dismiss-without-ring'; manager.notifyFromProtocol(id, { source: 'OSC 9', title: null, body: 'Build finished' }); - manager.attend(id); - - manager.applyTerminalSemanticEvents(id, [ - { type: 'commandLine', commandLine: 'claude --resume' }, - { type: 'commandStart', source: 'osc633_E', startedAt: Date.now() }, - ]); - - expect(manager.getState(id)).toMatchObject({ - watchingEnabled: true, - todo: true, - attentionDismissedRing: true, - }); + manager.dismissAlert(id); + const quiet = manager.getState(id); + const states: string[] = []; + manager.onStateChange((changed) => states.push(changed)); manager.dismissAlert(id); - expect(manager.getState(id).attentionDismissedRing).toBe(false); + + expect(states).toEqual([]); + expect(manager.getState(id)).toEqual(quiet); }); it('protocol completion is suppressed while the user has attention', () => { @@ -1427,7 +1413,6 @@ describe('AlertManager in isolation', () => { status: 'WATCHING_DISABLED', todo: true, notification: { source: 'OSC 9', title: null, body: 'Build finished' }, - attentionDismissedRing: false, awaited: false, }); }); diff --git a/lib/src/lib/alert-manager.ts b/lib/src/lib/alert-manager.ts index c41a9dacf..e1f3303ac 100644 --- a/lib/src/lib/alert-manager.ts +++ b/lib/src/lib/alert-manager.ts @@ -159,8 +159,6 @@ export interface AlertState { watchingEnabled: boolean; todo: TodoState; notification: ActivityNotification | null; - /** Used by the bell transition table to detect a post-attention dismiss */ - attentionDismissedRing: boolean; /** At least one `dor await` is parked on this Session. Never persisted. */ awaited: boolean; /** @@ -176,7 +174,6 @@ export const DEFAULT_ALERT_STATE: AlertState = { watchingEnabled: false, todo: false, notification: null, - attentionDismissedRing: false, awaited: false, ringSeq: 0, }; @@ -206,7 +203,6 @@ interface AlertEntry { pendingCommandLine: string | null; todo: TodoState; notification: ActivityNotification | null; - attentionDismissedRing: boolean; /** Latest terminal notification deferred behind animation; never public or persisted. */ deferredNotification: ActivityNotification | null; deferredNotificationTimer: ReturnType | null; @@ -600,8 +596,8 @@ export class AlertManager { /** * Consume the ring an await arriving right now would resolve on, if any. - * Only that track's latch is released: TODO, its notification detail, and - * `attentionDismissedRing` are the human's and stay untouched. + * Only that track's latch is released: TODO and its notification detail are + * the human's and stay untouched. * * Two of the three are gated, because their latches outlive the fact they * describe. @@ -1062,10 +1058,7 @@ export class AlertManager { const entry = this.getOrCreateEntry(id); this.setAttention(id); - if (this.clearAllRingsIfActive(entry)) { - entry.attentionDismissedRing = true; - entry.todo = true; - } + if (this.clearAllRingsIfActive(entry)) entry.todo = true; this.markCommandExitSeen(entry); this.notify(id); } @@ -1087,14 +1080,11 @@ export class AlertManager { const entry = this.entries.get(id); if (!entry) return; - const dismissed = this.clearAllRingsIfActive(entry); - if (dismissed) entry.todo = true; - // The flag exists so the next bell click opens the dialog instead of - // silently changing a rule; an explicit dismiss *is* that next click. - const hadFlag = entry.attentionDismissedRing; - entry.attentionDismissedRing = false; - - if (dismissed || hadFlag) this.notify(id); + // Dismissing a ring leaves the TODO behind, so the summons is not lost; on a + // Session with nothing ringing there is nothing to do. + if (!this.clearAllRingsIfActive(entry)) return; + entry.todo = true; + this.notify(id); } // --- Todo controls --- @@ -1138,7 +1128,6 @@ export class AlertManager { watchingEnabled: this.isWatching(entry), todo: entry.todo, notification: entry.notification, - attentionDismissedRing: entry.attentionDismissedRing, awaited: (this.awaits.get(id)?.waiters.size ?? 0) > 0, ringSeq: entry.ringSeq, episode: this.hasActiveRing(entry) ? entry.episode : null, @@ -1342,7 +1331,6 @@ export class AlertManager { pendingCommandLine: null, todo: false, notification: null, - attentionDismissedRing: false, deferredNotification: null, deferredNotificationTimer: null, }; @@ -1371,7 +1359,6 @@ function alertStatesEqual(a: AlertState, b: AlertState): boolean { a.status !== b.status || a.watchingEnabled !== b.watchingEnabled || a.todo !== b.todo - || a.attentionDismissedRing !== b.attentionDismissedRing || a.awaited !== b.awaited || a.ringSeq !== b.ringSeq || a.episode?.id !== b.episode?.id diff --git a/lib/src/lib/session-activity-store.ts b/lib/src/lib/session-activity-store.ts index 085d16d87..038e30598 100644 --- a/lib/src/lib/session-activity-store.ts +++ b/lib/src/lib/session-activity-store.ts @@ -1,26 +1,13 @@ import { createAlertEpisode } from './alert-episode'; -import type { AlertState, SessionStatus } from './alert-manager'; +import type { AlertState } from './alert-manager'; import type { AlertStateDetail } from './platform/types'; import { applyAlertSettingsFromHost, publishAlertSettings } from './alert-settings'; import { toPersistedAlertState, type PersistedAlertState, type PersistedPane } from './session-types'; import { getPlatform } from './platform'; -import { getRunningCommandArgv0 } from './terminal-state-store'; -import { - applyWatchedCommandsFromHost, - isCommandWatched, - publishWatchedCommands, - setCommandWatched, -} from './watched-commands'; +import { applyWatchedCommandsFromHost, publishWatchedCommands } from './watched-commands'; import { registry } from './terminal-store'; -/** - * What the bell click resolved to, so the caller knows whether to open the - * alert dialog. `no-command` means the pane is at a prompt: WATCHING is keyed - * on the running command, so there is nothing to enable. - */ -export type AlertButtonActionResult = 'enabled' | 'disabled' | 'dismissed' | 'menu' | 'no-command' | 'noop'; - -export type ActivityState = Omit; +export type ActivityState = AlertState; export const DEFAULT_ACTIVITY_STATE: ActivityState = { status: 'WATCHING_DISABLED', @@ -35,8 +22,7 @@ const activityListeners = new Set<(changedId?: string) => void>(); let cachedSnapshot: Map | null = null; // Terminal activity keeps the same home before and after xterm initialization. -// The dismissal flag belongs to the bell action, not the public UI snapshot. -const terminalActivity = new Map(); +const terminalActivity = new Map(); // Browser surfaces have no host alert stream. Keep their TODO separate so a // terminal taking the same id starts from its own activity, and clearing @@ -76,7 +62,7 @@ export function getActivity(id: string): ActivityState { } function readActivity(id: string): ActivityState | null { - return terminalActivity.get(id)?.state + return terminalActivity.get(id) ?? (registry.has(id) ? DEFAULT_ACTIVITY_STATE : localSurfaceActivity.get(id) ?? null); } @@ -86,17 +72,13 @@ export function getLivePersistedAlertState(id: string): PersistedAlertState | nu /** Install a host snapshot, including one received before xterm initialization. */ export function setTerminalActivity(id: string, state: Partial): void { - const { attentionDismissedRing = false, ...activity } = state; - const previous = terminalActivity.get(id)?.state; + const previous = terminalActivity.get(id); // Older hosts and local fixtures have no episode field. Hydrate their status // edges here; consumers still seed first-observed rings without delivery. const episode = state.status === 'ALERT_RINGING' ? state.episode ?? (previous?.status === 'ALERT_RINGING' ? previous.episode : null) ?? createAlertEpisode() : null; - terminalActivity.set(id, { - state: { ...DEFAULT_ACTIVITY_STATE, ...activity, episode }, - attentionDismissedRing, - }); + terminalActivity.set(id, { ...DEFAULT_ACTIVITY_STATE, ...state, episode }); notifyActivityListeners(id); } @@ -168,53 +150,8 @@ export function initAlertStateReceiver(): void { publishAlertSettings(); } -/** - * The bell-button transition table (`docs/specs/alert.md` -> UI Contract). This - * is the only copy: WATCHING is a rule keyed on the foreground command's name, - * so enabling and disabling both resolve to a rule-set edit, and the manager - * learns about it through a command-level mutation like any other rule change. - */ -export function dismissOrToggleAlert(id: string, displayedStatus: SessionStatus): AlertButtonActionResult { - if (displayedStatus === 'ALERT_RINGING') { - dismissSessionAlert(id); - return 'dismissed'; - } - - // An attention-based dismissal leaves a flag behind so this next click opens - // the dialog rather than silently editing a rule. - if (terminalActivity.get(id)?.attentionDismissedRing) { - dismissSessionAlert(id); - return 'dismissed'; - } - - // Everything else is "turn the rule for the running command on or off". - const argv0 = getRunningCommandArgv0(id); - if (!argv0) return 'no-command'; - - if (isCommandWatched(argv0)) { - setCommandWatched(argv0, false); - return 'disabled'; - } - - // A protocol/command-exit alarm needs no rule, so clicking through one would - // enable WATCHING by surprise. Show the detail dialog instead. - if (displayedStatus === 'OSC_NOTIF_BUSY' || displayedStatus === 'COMMAND_EXIT_ARMED') return 'menu'; - - setCommandWatched(argv0, true); - return 'enabled'; -} - -/** Turn the rule for whatever `id` is running on/off; no-op at a prompt. */ -export function toggleSessionAlert(id: string): void { - const argv0 = getRunningCommandArgv0(id); - if (argv0) setCommandWatched(argv0, !isCommandWatched(argv0)); -} - -export function disableSessionAlert(id: string): void { - const argv0 = getRunningCommandArgv0(id); - if (argv0) setCommandWatched(argv0, false); -} - +/** The whole of the alert action: a ring goes quiet (leaving its TODO) and the + * caller opens the terminal context (`docs/specs/alert.md` -> Pane Header). */ export function dismissSessionAlert(id: string): void { getPlatform().alertDismiss(id); } diff --git a/lib/src/lib/terminal-registry.alert.test.ts b/lib/src/lib/terminal-registry.alert.test.ts index 8af2b8387..65f083a74 100644 --- a/lib/src/lib/terminal-registry.alert.test.ts +++ b/lib/src/lib/terminal-registry.alert.test.ts @@ -117,8 +117,6 @@ import { disposeAllSessions, disposeSession, unmountElement, - disableSessionAlert, - dismissOrToggleAlert, dismissSessionAlert, focusSession, registerSurfaceFocusHandle, @@ -139,7 +137,6 @@ import { restoreTerminal, setPendingShellOpts, subscribeToActivity, - toggleSessionAlert, toggleSessionTodo, } from './terminal-registry'; import { pasteFilePaths } from './clipboard'; @@ -258,10 +255,10 @@ function runCommand(id: string, commandLine = 'longtask'): void { fakePlatform.sendOutput(id, `\x1b]633;E;${commandLine}\x07\x1b]633;C\x07`); } -/** Run `commandLine` and turn its WATCHING rule on, as the bell would. */ +/** Run `commandLine` and turn its WATCHING rule on, as the terminal context would. */ function enableAlert(id: string, commandLine = 'longtask'): void { runCommand(id, commandLine); - toggleSessionAlert(id); + setCommandWatched(commandLine.split(' ')[0], true); expect(getActivity(id).watchingEnabled).toBe(true); } @@ -396,19 +393,19 @@ describe('terminal-registry alert behavior', () => { expect(getActivitySnapshot().has(id)).toBe(false); }); - it('preserves early attention dismissal', () => { + it('keeps the TODO an early attention dismissal left behind', () => { const id = 'early-attention-dismissal'; fakePlatform.spawnPty(id); fakePlatform.sendOutput(id, '\x07'); expect(getActivity(id).status).toBe('ALERT_RINGING'); fakePlatform.alertAttend(id); - expect(getActivity(id).status).toBe('WATCHING_DISABLED'); + expect(getActivity(id)).toMatchObject({ status: 'WATCHING_DISABLED', todo: true }); resumeTerminal(id, null, { alive: true }); - expect(dismissOrToggleAlert(id, 'WATCHING_DISABLED')).toBe('dismissed'); - // The explicit dismissal consumes the flag; a second click is at a prompt. - expect(dismissOrToggleAlert(id, 'WATCHING_DISABLED')).toBe('no-command'); + // Nothing is ringing any more, so the alert action changes nothing. + dismissSessionAlert(id); + expect(getActivity(id)).toMatchObject({ status: 'WATCHING_DISABLED', todo: true }); }); it('retains a resumed exited Session TODO until disposal', () => { @@ -767,7 +764,7 @@ describe('terminal-registry alert behavior', () => { enableAlert(id); driveToRingingNeedsAttention(id); - disableSessionAlert(id); + setCommandWatched('longtask', false); expect(getActivity(id)).toMatchObject({ status: 'WATCHING_DISABLED', @@ -1153,13 +1150,13 @@ describe('terminal-registry alert behavior', () => { }); }); - it('disabling alerts while ringing does not turn TODO on', () => { + it('removing the rule while ringing does not turn TODO on', () => { const id = 'disable-no-todo'; createSession(id); enableAlert(id); driveToRingingNeedsAttention(id); - disableSessionAlert(id); + setCommandWatched('longtask', false); expect(getActivity(id)).toMatchObject({ status: 'WATCHING_DISABLED', @@ -1167,92 +1164,55 @@ describe('terminal-registry alert behavior', () => { }); }); - it('alert button enables alerts from WATCHING_DISABLED', () => { - const id = 'alert-button-enable'; - createSession(id); - runCommand(id); - - expect(dismissOrToggleAlert(id, 'WATCHING_DISABLED')).toBe('enabled'); - - expect(getActivity(id)).toMatchObject({ - status: 'NOTHING_TO_SHOW', - todo: false, - }); - }); - - it('alert button reports no-command at a prompt instead of enabling', () => { - const id = 'alert-button-no-command'; - createSession(id); - - // WATCHING is keyed on the running command; with nothing running there is - // no rule to create, so the header opens the dialog to explain that. - expect(dismissOrToggleAlert(id, 'WATCHING_DISABLED')).toBe('no-command'); - expect(getActivity(id).watchingEnabled).toBe(false); - expect(getWatchedCommands()).toEqual([]); - }); - - it('alert button turns the rule on for every session running that command', () => { - const alpha = 'alert-button-spread-a'; - const beta = 'alert-button-spread-b'; + it('a rule turns watching on for every session running that command', () => { + const alpha = 'alert-rule-spread-a'; + const beta = 'alert-rule-spread-b'; createSession(alpha); createSession(beta); runCommand(alpha, 'claude --resume'); runCommand(beta, '/usr/local/bin/claude'); - expect(dismissOrToggleAlert(alpha, 'WATCHING_DISABLED')).toBe('enabled'); + setCommandWatched('claude', true); expect(getWatchedCommands()).toEqual(['claude']); expect(getActivity(alpha).watchingEnabled).toBe(true); expect(getActivity(beta).watchingEnabled).toBe(true); - // Turning it off anywhere turns it off everywhere. - expect(dismissOrToggleAlert(beta, 'NOTHING_TO_SHOW')).toBe('disabled'); + // Removing it anywhere removes it everywhere. + setCommandWatched('claude', false); expect(getActivity(alpha).watchingEnabled).toBe(false); expect(getActivity(beta).watchingEnabled).toBe(false); }); - it('alert button disables alerts from enabled non-ringing states', () => { - const id = 'alert-button-disable'; + it('the alert action never edits a rule, whatever is running', () => { + const id = 'alert-action-keeps-rule'; createSession(id); enableAlert(id); driveToBusy(id); - dismissOrToggleAlert(id, 'BUSY'); + dismissSessionAlert(id); - expect(getActivity(id)).toMatchObject({ - status: 'WATCHING_DISABLED', - todo: false, - }); + expect(getWatchedCommands()).toEqual(['longtask']); + expect(getActivity(id)).toMatchObject({ status: 'BUSY', todo: false }); }); - it('alert button dismisses ringing alerts and turns TODO on', () => { - const id = 'alert-button-dismiss'; + it('the alert action at a prompt creates no rule', () => { + const id = 'alert-action-no-command'; createSession(id); - enableAlert(id); - driveToRingingNeedsAttention(id); - dismissOrToggleAlert(id, 'ALERT_RINGING'); + dismissSessionAlert(id); - expect(getActivity(id)).toMatchObject({ - status: 'NOTHING_TO_SHOW', - todo: true, - }); + expect(getActivity(id).watchingEnabled).toBe(false); + expect(getWatchedCommands()).toEqual([]); }); - it('clicking a bell rendered as ringing does not disable alerts after attention already reset it', () => { - const id = 'displayed-ringing-dismiss'; + it('the alert action dismisses ringing alerts and turns TODO on', () => { + const id = 'alert-action-dismiss'; createSession(id); enableAlert(id); - driveToRingingNeedsAttention(id); - markSessionAttention(id); - - expect(getActivity(id)).toMatchObject({ - status: 'NOTHING_TO_SHOW', - todo: true, - }); - dismissOrToggleAlert(id, 'ALERT_RINGING'); + dismissSessionAlert(id); expect(getActivity(id)).toMatchObject({ status: 'NOTHING_TO_SHOW', @@ -1260,8 +1220,8 @@ describe('terminal-registry alert behavior', () => { }); }); - it('a bell click immediately after attention clears ringing is treated as a dismiss, not disable', () => { - const id = 'recent-ringing-dismiss'; + it('the alert action leaves a Session attention already quieted alone', () => { + const id = 'displayed-ringing-dismiss'; createSession(id); enableAlert(id); @@ -1273,7 +1233,9 @@ describe('terminal-registry alert behavior', () => { todo: true, }); - expect(dismissOrToggleAlert(id, 'NOTHING_TO_SHOW')).toBe('dismissed'); + dismissSessionAlert(id); + + expect(getWatchedCommands()).toEqual(['longtask']); expect(getActivity(id)).toMatchObject({ status: 'NOTHING_TO_SHOW', todo: true, diff --git a/lib/src/lib/terminal-registry.ts b/lib/src/lib/terminal-registry.ts index bb1a5c908..ede490c5e 100644 --- a/lib/src/lib/terminal-registry.ts +++ b/lib/src/lib/terminal-registry.ts @@ -1,7 +1,7 @@ export type { SessionStatus } from './alert-manager'; export type { TodoState } from './alert-manager'; export type { AlertSpeechState } from './alert-speech-state'; -export type { ActivityState, AlertButtonActionResult } from './session-activity-store'; +export type { ActivityState } from './session-activity-store'; export type { TerminalEntry, TerminalOverlayDims } from './terminal-store'; export type { CommandRun, @@ -20,8 +20,6 @@ export { clearSessionAttention, clearSessionTodo, DEFAULT_ACTIVITY_STATE, - disableSessionAlert, - dismissOrToggleAlert, dismissSessionAlert, getActivity, getActivitySnapshot, @@ -32,7 +30,6 @@ export { setTerminalActivity, restoreBrowserSurfaceTodo, subscribeToActivity, - toggleSessionAlert, toggleSessionTodo, } from './session-activity-store'; diff --git a/lib/src/stories/BrowserChromeHeader.stories.tsx b/lib/src/stories/BrowserChromeHeader.stories.tsx index 377363293..88ac4e4d7 100644 --- a/lib/src/stories/BrowserChromeHeader.stories.tsx +++ b/lib/src/stories/BrowserChromeHeader.stories.tsx @@ -37,7 +37,7 @@ import { setDevServerResolution } from '../components/wall/agent-browser-ports'; const loggingActions: WallActions = { onKill: () => console.log('[story] kill'), onMinimize: () => console.log('[story] minimize'), - onAlertButton: () => 'noop', + onAlertButton: () => {}, onToggleTodo: () => {}, onSplitH: () => console.log('[story] split left/right'), onSplitV: () => console.log('[story] split top/bottom'), diff --git a/lib/src/stories/MouseHeaderIcon.stories.tsx b/lib/src/stories/MouseHeaderIcon.stories.tsx index b51cc5c02..bbf7b14d9 100644 --- a/lib/src/stories/MouseHeaderIcon.stories.tsx +++ b/lib/src/stories/MouseHeaderIcon.stories.tsx @@ -22,7 +22,7 @@ const SESSION_ID = 'mouse-story'; const noopActions: WallActions = { onKill: () => {}, onMinimize: () => {}, - onAlertButton: () => 'noop', + onAlertButton: () => {}, onToggleTodo: () => {}, onSplitH: () => {}, onSplitV: () => {}, diff --git a/lib/src/stories/ShellCwd.stories.tsx b/lib/src/stories/ShellCwd.stories.tsx index d2795cf8b..754cae610 100644 --- a/lib/src/stories/ShellCwd.stories.tsx +++ b/lib/src/stories/ShellCwd.stories.tsx @@ -45,7 +45,7 @@ interface ShellCwdCase { const noopActions: WallActions = { onKill: () => {}, onMinimize: () => {}, - onAlertButton: () => 'noop', + onAlertButton: () => {}, onToggleTodo: () => {}, onSplitH: () => {}, onSplitV: () => {}, diff --git a/lib/src/stories/TerminalPaneHeader.stories.tsx b/lib/src/stories/TerminalPaneHeader.stories.tsx index 8a1a2fa24..866e2b6b5 100644 --- a/lib/src/stories/TerminalPaneHeader.stories.tsx +++ b/lib/src/stories/TerminalPaneHeader.stories.tsx @@ -24,7 +24,7 @@ const SESSION_ID = 'tab-story'; const noopActions: WallActions = { onKill: () => {}, onMinimize: () => {}, - onAlertButton: () => 'noop', + onAlertButton: () => {}, onToggleTodo: () => {}, onSplitH: () => {}, onSplitV: () => {}, @@ -236,9 +236,8 @@ async function openAlertRightClickDialog() { } /** - * Hover the bell so its tooltip renders — the tooltip is what carries the - * command-scoped wording, e.g. `Alert on all "claude"` vs `Alerts are per - * command`. + * Hover the bell so its tooltip renders — the tooltip is what names the action, + * e.g. `[a] Dismiss alert` vs `[a] Alert settings`. * * Hover rather than focus: a programmatic `.focus()` does not reliably drive * React's `onFocus` here, while `mouseover` is exactly what React synthesizes diff --git a/scripts/spec-word-budgets.json b/scripts/spec-word-budgets.json index 5708c5379..670f32fe1 100644 --- a/scripts/spec-word-budgets.json +++ b/scripts/spec-word-budgets.json @@ -2,7 +2,7 @@ "AGENTS.md": 3400, "SECURITY.md": 200, "SELF_HOST.md": 6200, - "docs/specs/alert.md": 7250, + "docs/specs/alert.md": 7200, "docs/specs/auto-update.md": 1200, "docs/specs/deploy.md": 1900, "docs/specs/dor-browser.md": 4700, diff --git a/vscode-ext/README.md b/vscode-ext/README.md index ed9fba01b..f4903bfda 100644 --- a/vscode-ext/README.md +++ b/vscode-ext/README.md @@ -34,7 +34,7 @@ Dormouse can owe you attention in three independent ways. Two of them need no se **A command finishes while you are away.** If a foreground command was running while you were watching, you left, and it exited after you had been gone a while, that pane is ringing when you come back. Also no configuration. -**A watched command goes quiet.** This one is opt-in, and it is the one for coding agents. Click the bell in a pane running `claude` — or press `a` in command mode — and Dormouse watches *that command name*. Every pane running `claude` is then watched, the ones open now and the ones you open later. When a watched command's output goes busy and then falls quiet while you are not looking, it rings. +**A watched command goes quiet.** This one is opt-in, and it is the one for coding agents. Right-click the header of a pane running `claude` — or press `a` in command mode — and turn on **Watch all claude commands**; Dormouse then watches *that command name*. Every pane running `claude` is then watched, the ones open now and the ones you open later. When a watched command's output goes busy and then falls quiet while you are not looking, it rings. Dormouse never guesses which commands deserve an alert. Watching is a rule you create on a command name, and turning it off anywhere removes it everywhere. diff --git a/vscode-ext/test/session-state.test.ts b/vscode-ext/test/session-state.test.ts index 57a976075..f2ae9a487 100644 --- a/vscode-ext/test/session-state.test.ts +++ b/vscode-ext/test/session-state.test.ts @@ -51,7 +51,6 @@ describe('VS Code session alert persistence', () => { status: 'ALERT_RINGING', watchingEnabled: true, todo: true, - attentionDismissedRing: true, awaited: true, ringSeq: 7, })], diff --git a/website/src/lib/__snapshots__/tut-runner.test.ts.snap b/website/src/lib/__snapshots__/tut-runner.test.ts.snap index de3f65341..6d49988bb 100644 --- a/website/src/lib/__snapshots__/tut-runner.test.ts.snap +++ b/website/src/lib/__snapshots__/tut-runner.test.ts.snap @@ -64,9 +64,9 @@ exports[`TutRunner snapshots > renders the alert section with all items incomple Esc to go back ● Alert me whenever longtask runs - Press s to start a fake longtask, then click that pane's bell (or select - it and press a). Alerts belong to the command, not the tab — the bell - says "Alert on all longtask". + Press s to start a fake longtask, then right-click that pane's header + (or select it and press a) and turn on "Watch all longtask commands". + Alerts belong to the command, not the tab. · The rule covers every pane running that command · The bell tilts while the command works · It rings when the command goes quiet diff --git a/website/src/lib/tut-items.ts b/website/src/lib/tut-items.ts index 1aeba1670..5f4d8084a 100644 --- a/website/src/lib/tut-items.ts +++ b/website/src/lib/tut-items.ts @@ -198,12 +198,12 @@ export const DESKTOP_SECTIONS: readonly Section[] = [ { id: 'al-watch-cmd', title: 'Alert me whenever `longtask` runs', - hint: 'Press `s` to start a fake `longtask`, then click that pane\'s bell (or select it and press `a`). Alerts belong to the command, not the tab — the bell says "Alert on all longtask".', + hint: 'Press `s` to start a fake `longtask`, then right-click that pane\'s header (or select it and press `a`) and turn on "Watch all longtask commands". Alerts belong to the command, not the tab.', }, { id: 'al-spreads', title: 'The rule covers every pane running that command', - hint: 'Both fake tasks light up from the one bell you clicked. Any pane you open later that runs `longtask` will watch too, with no extra clicks.', + hint: 'Both fake tasks light up from the one rule you turned on. Any pane you open later that runs `longtask` will watch too, with no extra clicks.', }, { id: 'al-busy', @@ -228,7 +228,7 @@ export const DESKTOP_SECTIONS: readonly Section[] = [ { id: 'al-todo-manual', title: 'Add a TODO by hand', - hint: 'Press `t` in command mode, or right-click the bell.', + hint: 'Press `t` in command mode, or right-click the header and use the TODO switch.', }, { id: 'al-notif', diff --git a/website/src/pages/PlaygroundDesktop.tsx b/website/src/pages/PlaygroundDesktop.tsx index 08c9faafb..ca81535bd 100644 --- a/website/src/pages/PlaygroundDesktop.tsx +++ b/website/src/pages/PlaygroundDesktop.tsx @@ -182,8 +182,8 @@ function PlaygroundDesktopExperience() { getInactivityTimeoutMs: () => alertSettings.getAlertSettings().inactivityTimeoutMs, // WATCHING is keyed on the running command, so the demo has to // report one through shell integration. Both alert panes run the - // same fake `longtask`, which is what lets one bell click light - // up the other pane (docs/specs/alert.md). + // same fake `longtask`, which is what lets one rule light up the + // other pane (docs/specs/alert.md). onTriggerBusyDemo: (durationMs, commandMs) => { busyDemoDisposeRef.current?.(); if (busyDemoFinishTimerRef.current !== null) { From c9f396f5fbd3e04b492c4bf19318971de935671e Mon Sep 17 00:00:00 2001 From: Ned Twigg Date: Sat, 19 Sep 2026 23:58:42 -0700 Subject: [PATCH 2/2] Open the terminal context from the keyboard through the Wall, not a synthetic click The a and > keys now call one openTerminalContext hook on the keyboard context instead of clicking the header's bell or dispatching a fake contextmenu event, so the keyboard layer no longer depends on the header having a button. onAlertButton was a pass-through to dismissSessionAlert and is gone; the header calls the store directly, the alert label is one boolean, DEFAULT_ACTIVITY_STATE aliases DEFAULT_ALERT_STATE, and the watched-command remove control names what it does. Co-Authored-By: Claude Fable 5.1 --- TUTORIAL_MOCKUP.md | 8 ++-- docs/specs/layout.md | 2 +- lib/src/components/MobileWall.tsx | 9 ++--- lib/src/components/SettingsDialog.tsx | 6 +-- lib/src/components/Wall.tsx | 5 +-- lib/src/components/WatchedCommandList.tsx | 14 ++----- .../components/wall/TerminalPaneHeader.tsx | 23 +++++------ .../keyboard/handle-pane-shortcuts.test.ts | 32 ++++++++++----- .../wall/keyboard/handle-pane-shortcuts.ts | 40 +++++-------------- lib/src/components/wall/keyboard/types.ts | 3 ++ lib/src/components/wall/wall-context.tsx | 4 -- lib/src/components/wall/wall-test-utils.ts | 1 - lib/src/lib/session-activity-store.ts | 11 +---- lib/src/lib/terminal-registry.alert.test.ts | 3 +- .../stories/BrowserChromeHeader.stories.tsx | 1 - lib/src/stories/MouseHeaderIcon.stories.tsx | 1 - lib/src/stories/ShellCwd.stories.tsx | 1 - .../stories/TerminalPaneHeader.stories.tsx | 1 - 18 files changed, 61 insertions(+), 104 deletions(-) diff --git a/TUTORIAL_MOCKUP.md b/TUTORIAL_MOCKUP.md index ea45d823d..da0193753 100644 --- a/TUTORIAL_MOCKUP.md +++ b/TUTORIAL_MOCKUP.md @@ -35,8 +35,8 @@ Four beats. Beat 2 is the section; the rest is scaffolding around it. | id | title | hint | |---|---|---| -| `al-watch-cmd` | Alert me whenever `longtask` runs | Press `s` to start a fake `longtask`, then click that pane's bell. The bell says *Alert on all "longtask"* — you are describing a command, not flagging a tab. | -| `al-spreads` | One rule, every pane running it | The second fake task lit up too, and you never touched its bell. Any pane you open later that runs `longtask` will watch as well. | +| `al-watch-cmd` | Alert me whenever `longtask` runs | Press `s` to start a fake `longtask`, then right-click that pane's header (or press `a`) and turn on *Watch all longtask commands* — you are describing a command, not flagging a tab. | +| `al-spreads` | One rule, every pane running it | The second fake task lit up too, and you never touched it. Any pane you open later that runs `longtask` will watch as well. | ## Beat 2 — Attention @@ -77,9 +77,9 @@ Both are rule-free, and both obey the same attention rule as Beat 2. | id | title | hint | |---|---|---| -| `al-todo-auto` | A dismissed ring leaves a TODO | Click the bell, or just select the pane. The ring goes away but a TODO stays, so an alert you waved away mid-thought does not vanish. | +| `al-todo-auto` | A dismissed ring leaves a TODO | Press `a`, or just select the pane. The ring goes away but a TODO stays, so an alert you waved away mid-thought does not vanish. | | `al-todo-clear` | Press `Enter` in the pane to clear it | Dealing with the pane is what clears the reminder — not looking at it. | -| `al-todo-manual` | Add a TODO by hand | Press `t`, or right-click the bell. Same tag, no alert required. | +| `al-todo-manual` | Add a TODO by hand | Press `t`, or right-click the header. Same tag, no alert required. | ## Full step list (10) diff --git a/docs/specs/layout.md b/docs/specs/layout.md index a82c3e2c1..d082e097f 100644 --- a/docs/specs/layout.md +++ b/docs/specs/layout.md @@ -62,7 +62,7 @@ The label is the `DerivedHeader` from `deriveHeader(...)`; `docs/specs/terminal- **Must float the context inside its source Pane with a one-rem inset on every side**, overlapping the header, with a theme-derived edge and raised shadow. Render it in the Lath leaf's overlay slot, outside the body's clipping box, so it follows the leaf's layout without remounting the helper. Keep one context per Wall. Outside pointer press and explicit close dismiss it. No separate context heading or clipboard toolbar is shown. -**Must reveal the context from the opening pointer position, clamped to its bounds, over 320ms.** Alert activation uses the alert button center; command-mode `>` uses the header's bottom-left; openings without a position use the context's top-left. Keep final layout dimensions throughout the reveal. Start helper creation, settings reads, and port scanning immediately on mount; fade mounted content, including detail dialogs, in over 140ms after 160ms. Reduced motion or disabled layout animation skips both animations and the delay. +**Must reveal the context from the opening pointer position, clamped to its bounds, over 320ms.** Alert-button activation uses the button center; command-mode `a` and `>` use the header's bottom-left; openings without a position use the context's top-left. Keep final layout dimensions throughout the reveal. Start helper creation, settings reads, and port scanning immediately on mount; fade mounted content, including detail dialogs, in over 140ms after 160ms. Reduced motion or disabled layout animation skips both animations and the delay. **Must contract dismissals toward the opening origin over 180ms, fading content over 100ms**, starting from the current reveal when interrupted. Make the closing context inert and pause helper polling immediately; release focus without waiting for removal. Reopening cancels pending removal. Reduced motion dismisses immediately; promotion, source removal, and replacement by another context retain their immediate lifecycle transitions. diff --git a/lib/src/components/MobileWall.tsx b/lib/src/components/MobileWall.tsx index 351ac314a..d3c4f2861 100644 --- a/lib/src/components/MobileWall.tsx +++ b/lib/src/components/MobileWall.tsx @@ -20,7 +20,6 @@ import { setTerminalUserTitle, subscribeToActivity, subscribeToTerminalPaneState, - type SessionStatus, } from '../lib/terminal-registry'; import { buildAppTitleResolver, @@ -48,10 +47,6 @@ export interface MobileWallProps { const DEFAULT_MOBILE_SESSION: MobileWallSession = { id: 'mobile-pane' }; -// Mobile has no terminal context, so dismissing a ring is the button's whole -// action; it never edits a rule (`docs/specs/alert.md` -> Pane Header). -const alertButtonLabelFor = (status: SessionStatus): string => - status === 'ALERT_RINGING' ? 'Dismiss alert' : 'Alert status'; export function useMobileWallSessionItems( sessions: MobileWallSession[], @@ -179,7 +174,9 @@ function MobileWallHeader({ }) { const status = session.status ?? 'WATCHING_DISABLED'; const todoPill = useTodoPillContent(session.todo === true); - const alertButtonLabel = alertButtonLabelFor(status); + // Mobile has no terminal context, so dismissing a ring is the button's whole + // action; it never edits a rule (`docs/specs/alert.md` -> Pane Header). + const alertButtonLabel = status === 'ALERT_RINGING' ? 'Dismiss alert' : 'Alert status'; const showTodoPill = todoPill.visible; return ( diff --git a/lib/src/components/SettingsDialog.tsx b/lib/src/components/SettingsDialog.tsx index cd83ca2fe..50dbe0d1b 100644 --- a/lib/src/components/SettingsDialog.tsx +++ b/lib/src/components/SettingsDialog.tsx @@ -83,10 +83,8 @@ function describePushTargets(push: PushDevicesState, remoteControlBelow: boolean * (`lib/src/lib/shell-store.ts`), then the alarm settings * (`docs/specs/alert.md` -> Alarm settings). * - * Rules are removable here but not addable: WATCHING is keyed on a running - * command's name, so a rule is created in the terminal context of a Pane - * running it. This dialog is the one place a rule set on a since-closed Pane - * can be found and removed. + * Rules are removable here but not addable (`docs/specs/alert.md` -> Settings + * dialog). */ export function SettingsDialog({ onClose }: { onClose: () => void }) { const watched = useSyncExternalStore(subscribeToWatchedCommands, getWatchedCommandsSnapshot); diff --git a/lib/src/components/Wall.tsx b/lib/src/components/Wall.tsx index de0f65355..cd003fd7f 100644 --- a/lib/src/components/Wall.tsx +++ b/lib/src/components/Wall.tsx @@ -38,7 +38,6 @@ import { clearLocalSurfaceActivity, deriveSessionLabel, disposeSession, - dismissSessionAlert, focusSession, refitSession, markSessionAttention, @@ -1894,9 +1893,6 @@ export function Wall({ exitTerminalMode(); requestKill(id); }, - onAlertButton: (id: string) => { - dismissSessionAlert(id); - }, onToggleTodo: (id: string) => { toggleSessionTodo(id); }, @@ -2222,6 +2218,7 @@ export function Wall({ enterTerminalMode, exitTerminalMode, minimizePane, + openTerminalContext: (id, origin) => contextActions.open(id, { origin }), requestKill, acceptKill, rejectKill, diff --git a/lib/src/components/WatchedCommandList.tsx b/lib/src/components/WatchedCommandList.tsx index 1a3e2f5e3..7da6a91fc 100644 --- a/lib/src/components/WatchedCommandList.tsx +++ b/lib/src/components/WatchedCommandList.tsx @@ -7,16 +7,8 @@ import { subscribeToWatchedCommands, } from '../lib/terminal-registry'; -/** - * The app-global WATCHING rule set, with a remove control per rule - * (`docs/specs/alert.md` -> WATCHING Track). - * - * Rendered only by the Alarm settings dialog — the one place a rule set on a - * since-closed Pane can be found and removed. - * - * Rules are removable but not addable: WATCHING is keyed on a running command's - * name, so creating one stays the terminal context of a Pane running it. - */ +/** The app-global WATCHING rule set with a remove control per rule; rules are + * created elsewhere (`docs/specs/alert.md` -> Settings dialog). */ export function WatchedCommandList() { const watched = useSyncExternalStore(subscribeToWatchedCommands, getWatchedCommandsSnapshot); if (watched.length === 0) return null; @@ -28,7 +20,7 @@ export function WatchedCommandList() { {name}