diff --git a/docs/specs/alert.md b/docs/specs/alert.md index 2bff86d7c..f1dba6fe2 100644 --- a/docs/specs/alert.md +++ b/docs/specs/alert.md @@ -34,7 +34,7 @@ Public `status` is a projection — first match wins: 4. `COMMAND_EXIT_ARMED` if command-exit alerting is armed. 5. Otherwise `WATCHING_DISABLED`. -**Must identify each uninterrupted ringing interval with an `episode` id and start time.** The first track to latch creates it; the last track clearing ends it. Additional track latches advance presentation-only `ringSeq` without starting another delivery episode. **Never persist episodes.** Older host snapshots receive a renderer-local identity on their first ringing transition. +**Must identify each uninterrupted ringing interval with an `episode` id and start time.** The first track to latch creates it; the last track clearing ends it. Additional track latches join the episode; they never start another delivery episode or replay the alarm burst. **Never persist episodes.** Older host snapshots receive a renderer-local identity on their first ringing transition. Tests: `a second track latching mid-episode keeps the episode id` and `re-latching after all tracks clear starts a new episode` in `lib/src/lib/alert-manager.test.ts`. `awaited` sits beside `status`: true while at least one `dor await` is parked on the Session (Await). It is derived from live waiters and **never persisted**. @@ -373,8 +373,6 @@ Where it surfaces is host-specific: The header shows 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 `a`. **Never tint a ringing Session's header**: the Pane overlay already outlines it. Placement, sizing, and width tiers belong to `docs/specs/layout.md`. -`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`; rationale). - - **`a` on the selected Pane in command mode dismisses a ringing Session and opens the terminal context, whatever the status; it never edits a WATCHING rule.** - **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. @@ -388,7 +386,7 @@ The TODO pill always displays `TODO`; remote notification text belongs in previe **Must wear the alarm treatment on every ringing terminal Pane**, labelled only once the speech sink acts. **Must bound the unlabelled pulse to one finite burst per episode, never replayed by a remount; `SPEAKING` pulses for its utterance, `SPOKEN` never** (rationale). **`prefers-reduced-motion` keeps the strong static treatment and suppresses only the pulse**, as does `cfg.alert.ringingPaused` (rationale). The three rows, their layers, strengths, and sizing are inventoried by `docs/specs/layout.md` → Alarm overlay. -Source of truth: `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`; `AlertRingIndicator` in `lib/src/components/wall/AlertRingIndicator.tsx`; `alertRingRow`, `useAlertRingBurst`, `AlertRingInset` in `lib/src/components/alert-ring.tsx`. +Source of truth: `openEpisode` 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`; `AlertRingIndicator` in `lib/src/components/wall/AlertRingIndicator.tsx`; `alertRingRow`, `useAlertRingBurst`, `AlertRingInset` in `lib/src/components/alert-ring.tsx`. ### Door diff --git a/docs/specs/alert.rationale.md b/docs/specs/alert.rationale.md index 84811fa1c..6db04f3c1 100644 --- a/docs/specs/alert.rationale.md +++ b/docs/specs/alert.rationale.md @@ -104,14 +104,11 @@ Guarding only completion leaves a stale `start` free to replace the active utter ## Pane Header -**Why a counter, not the status.** Bounding the burst turned a continuous cue into an edge-triggered one, and the public status has no such edge: `hasActiveRing` ORs three independently latching tracks, so a second alert behind a latched one leaves `ALERT_RINGING` in place. `notification` is no better — `applyCommandExitRinging` deliberately preserves a richer protocol notification. With both unchanged, `alertStatesEqual` also judged the two states equal and never emitted, so the renderer could not have reacted even had it wanted to. `ringSeq` is the smallest thing that changes exactly once per latch. - **Why `SPEAKING` may pulse unbounded and `SPOKEN` may not.** An utterance is seconds long and stops on its own, so the pulse it carries is self-bounding. `SPOKEN` persists until the ring is attended, so animating it would be exactly the per-Session animation with no end that bounding the burst exists to remove. -**Why latches and not notifications.** Counting every ring rule instead would let a Session bell-ing in a loop emit one host→webview update per PTY chunk, each restarting a 3.2s burst that never finishes — the always-running animation the finite burst exists to remove. A latch advances the counter at most once while that track remains latched; after release, relatching is a fresh summons and may replay. That matches the model `deferOrDeliverNotification` already states: an existing ring is enrichment, not a fresh summons. A timestamp floor would bound notifications too, but it would put the CSS duration in the manager. **Why `cfg.alert.ringingPaused` suppresses the pulse.** It is the Chromatic freeze that pins the alarm; even a bounded animation could otherwise snapshot at an arbitrary phase during its first 2.6 seconds. -**Why the unlabelled treatment pulses once per episode.** An infinite per-Session animation is expensive, and the whole-Pane treatment covers far more surface than the retired bell icon did. With four focused panes wearing an infinite animation, three minutes cost 6.89 MB of embedder memory, 1,127 style recalculations, and 3.99 seconds of renderer CPU; pausing only those animations in the same loaded document reduced that to 0.13 MB, two recalculations, and 0.025 seconds. After bounding the burst, two consecutive three-minute windows each had zero live animations, one recalculation, under 0.40 MB of non-cumulative embedder drift, and at most 0.024 seconds of renderer CPU (measured in Chrome 150, 2026-09). A handful of cycles preserves the entry cue without leaving an animation running for the lifetime of an unattended alert. The episode — not a track latch — is the key because the episode is the summons the sinks already work from: a second track latching inside one enriches an alarm the user was already shown, and re-flashing the whole Pane for it would read as a new alarm. Running the burst off `episode.startedAt` rather than from mount makes the CSS clock a property of the episode, so minimize → reattach or a Workspace switch lands past an expired burst instead of replaying it. +**Why the unlabelled treatment pulses once per episode.** An infinite per-Session animation is expensive, and the whole-Pane treatment covers far more surface than the retired bell icon did. With four focused panes wearing an infinite animation, three minutes cost 6.89 MB of embedder memory, 1,127 style recalculations, and 3.99 seconds of renderer CPU; pausing only those animations in the same loaded document reduced that to 0.13 MB, two recalculations, and 0.025 seconds. After bounding the burst, two consecutive three-minute windows each had zero live animations, one recalculation, under 0.40 MB of non-cumulative embedder drift, and at most 0.024 seconds of renderer CPU (measured in Chrome 150, 2026-09). A handful of cycles preserves the entry cue without leaving an animation running for the lifetime of an unattended alert. The episode — not a track latch — is the key because the episode is the summons the sinks already work from: a second track latching inside one enriches an alarm the user was already shown, and re-flashing the whole Pane for it would read as a new alarm. Running the burst off `episode.startedAt` rather than from mount makes the CSS clock a property of the episode, so minimize → reattach or a Workspace switch lands past an expired burst instead of replaying it. A per-latch counter carried this edge while the burst hung off the retired bell, and it went with the bell once the episode became the only presentation edge. A Session BEL-ing in a loop still cannot restart the burst, because a track that is already latched does not re-latch. ## Text And Security diff --git a/lib/src/components/WorkspaceWindow.test.tsx b/lib/src/components/WorkspaceWindow.test.tsx index 91bbac858..6c4007e00 100644 --- a/lib/src/components/WorkspaceWindow.test.tsx +++ b/lib/src/components/WorkspaceWindow.test.tsx @@ -373,7 +373,8 @@ describe('WorkspaceWindow', () => { const first = getWorkspacesSnapshot().workspaces[0].id; await render(); setTerminalActivity('pane-a', { status: 'ALERT_RINGING' }); - const ringBefore = getActivitySnapshot().get('pane-a')!.ringSeq; + const episodeBefore = getActivitySnapshot().get('pane-a')!.episode; + expect(episodeBefore?.id).toBeTruthy(); const leafBefore = wallFor(first).querySelector('[data-lath-leaf="pane-a"]'); const paneBefore = wallFor(first).querySelector('[data-session-id="pane-a"]'); @@ -383,11 +384,11 @@ describe('WorkspaceWindow', () => { await flush(); // A switch flips a prop; it never unmounts a leaf, so nothing calls - // mountElement / resumeTerminal / restoreTerminal and `ringSeq` cannot - // advance (docs/specs/glossary.md → "Invariants" I8). + // mountElement / resumeTerminal / restoreTerminal and the delivery episode + // cannot restart (docs/specs/glossary.md → "Invariants" I8). expect(wallFor(first).querySelector('[data-lath-leaf="pane-a"]')).toBe(leafBefore); expect(wallFor(first).querySelector('[data-session-id="pane-a"]')).toBe(paneBefore); - expect(getActivitySnapshot().get('pane-a')!.ringSeq).toBe(ringBefore); + expect(getActivitySnapshot().get('pane-a')!.episode?.id).toBe(episodeBefore?.id); }); it('gives host New Terminal and the dialog hosts to the visible Workspace only', async () => { diff --git a/lib/src/lib/alert-manager.test.ts b/lib/src/lib/alert-manager.test.ts index 16312013a..2a2818e82 100644 --- a/lib/src/lib/alert-manager.test.ts +++ b/lib/src/lib/alert-manager.test.ts @@ -430,12 +430,12 @@ describe('AlertManager in isolation', () => { }); }); - // `docs/specs/alert.md` -> Pane Header. - it('counts a second track ringing behind an already-latched one', () => { - const id = 'ring-seq-cross-track'; - const seqs: number[] = []; + // `docs/specs/alert.md` -> Public State. + it('a second track latching mid-episode keeps the episode id', () => { + const id = 'episode-cross-track'; + const seen: string[] = []; manager.onStateChange((_id, state) => { - if (_id === id) seqs.push(state.ringSeq); + if (_id === id && state.episode) seen.push(state.episode.id); }); manager.attend(id); @@ -450,31 +450,38 @@ describe('AlertManager in isolation', () => { ]); const rung = manager.getState(id); expect(rung.status).toBe('ALERT_RINGING'); + expect(rung.episode?.id).toBeTruthy(); - // The command-exit track latches behind the protocol one. Everything else the - // renderer could have keyed on is unchanged across this ring. + // The command-exit track latches behind the protocol one: one enriched + // summons, so no consumer keyed on the episode may deliver a second time. manager.applyTerminalSemanticEvents(id, [{ type: 'commandFinish', exitCode: 0 }]); const again = manager.getState(id); expect(again.status).toBe(rung.status); expect(again.notification).toEqual(rung.notification); - expect(again.ringSeq).toBeGreaterThan(rung.ringSeq); - // And it has to reach subscribers: `alertStatesEqual` would otherwise call - // these two states equal and drop the update before it left the host. - expect(seqs).toContain(again.ringSeq); + expect(again.episode?.id).toBe(rung.episode?.id); + expect(new Set(seen)).toEqual(new Set([rung.episode!.id])); }); - // The counter is bounded by construction: a repeated notification on a track - // that is already ringing enriches the standing summons rather than raising a - // new one, so bell spam cannot restart the burst faster than it can play. - it('does not count a track that is already ringing', () => { - const id = 'ring-seq-same-track'; + it('re-latching after all tracks clear starts a new episode', () => { + const id = 'episode-restart'; const bell = { source: 'BEL', title: 'Terminal bell', body: null } as const; applyTerminalProtocolEvents(manager, id, [{ kind: 'notification', notification: bell }]); - const first = manager.getState(id).ringSeq; + const first = manager.getState(id).episode; + expect(first?.id).toBeTruthy(); + + // Bell spam on a track that is already ringing enriches the standing + // summons; it cannot raise a new episode, so nothing keyed on one replays. applyTerminalProtocolEvents(manager, id, [{ kind: 'notification', notification: bell }]); + expect(manager.getState(id).episode?.id).toBe(first!.id); + + manager.clearTodo(id); + expect(manager.getState(id).episode).toBeNull(); - expect(manager.getState(id).ringSeq).toBe(first); + applyTerminalProtocolEvents(manager, id, [{ kind: 'notification', notification: bell }]); + const second = manager.getState(id).episode; + expect(second?.id).toBeTruthy(); + expect(second?.id).not.toBe(first!.id); }); it('finishes an armed command-exit watch when the PTY exits without commandFinish', () => { diff --git a/lib/src/lib/alert-manager.ts b/lib/src/lib/alert-manager.ts index 4bd2f6353..51b504177 100644 --- a/lib/src/lib/alert-manager.ts +++ b/lib/src/lib/alert-manager.ts @@ -161,11 +161,6 @@ export interface AlertState { notification: ActivityNotification | null; /** At least one `dor await` is parked on this Session. Never persisted. */ awaited: boolean; - /** - * How many alarm tracks have latched on this Session, monotonic. Read only for - * change, never as a magnitude (`docs/specs/alert.md` -> Pane Header). - */ - ringSeq: number; } export const DEFAULT_ALERT_STATE: AlertState = { @@ -175,7 +170,6 @@ export const DEFAULT_ALERT_STATE: AlertState = { todo: false, notification: null, awaited: false, - ringSeq: 0, }; /** Three independent alarm tracks plus an always-on, non-latching detector. @@ -194,8 +188,6 @@ interface AlertEntry { * about the interval since the ring, which is only observable here. */ outputSinceWatchingRing: boolean; - /** Source of `AlertState.ringSeq`; see the field's contract there. */ - ringSeq: number; protocolStatus: ProtocolStatus; progress: ActiveProtocolProgress | null; commandExitStatus: CommandExitStatus; @@ -457,7 +449,7 @@ export class AlertManager { // it right now. The originating command key latches here so the ring // outlives the command that raised it. if (!this.isWatching(entry) || this.hasAttention(id)) break; - this.latchRing(entry, entry.watchingRingingCommand !== null); + this.openEpisode(entry); entry.watchingRingingCommand = entry.commandExitWatch?.argv0 ?? null; entry.outputSinceWatchingRing = false; this.notify(id); @@ -759,7 +751,7 @@ export class AlertManager { } private applyProtocolRinging(entry: AlertEntry, notification: ActivityNotification): void { - this.latchRing(entry, entry.protocolStatus === 'ALERT_RINGING'); + this.openEpisode(entry); entry.notification = notification; entry.todo = true; entry.protocolStatus = 'ALERT_RINGING'; @@ -886,7 +878,7 @@ export class AlertManager { displayCommand: string, exitCode: number | undefined, ): void { - this.latchRing(entry, entry.commandExitStatus === 'ALERT_RINGING'); + this.openEpisode(entry); entry.commandExitStatus = 'ALERT_RINGING'; entry.todo = true; // A protocol ring carries richer text; never overwrite it with the generic one. @@ -990,13 +982,12 @@ export class AlertManager { } /** - * Count one track latching. The mirror of `releaseRing`: a track that is - * already ringing is enrichment of the same summons, not a fresh one, so it - * does not advance the counter — see `deferOrDeliverNotification`. + * Open a delivery episode only when no track is ringing yet, so a track + * latching behind an already-ringing one enriches that same summons. Nothing + * closes one: `getState` masks a stale episode to `null` while no track rings. */ - private latchRing(entry: AlertEntry, wasRinging: boolean): void { + private openEpisode(entry: AlertEntry): void { if (!this.hasActiveRing(entry)) entry.episode = createAlertEpisode(); - if (!wasRinging) entry.ringSeq++; } /** Release one track's latched ring. Returns whether it was ringing. */ @@ -1131,7 +1122,6 @@ export class AlertManager { todo: entry.todo, notification: entry.notification, awaited: (this.awaits.get(id)?.waiters.size ?? 0) > 0, - ringSeq: entry.ringSeq, episode: this.hasActiveRing(entry) ? entry.episode : null, }; } @@ -1324,7 +1314,6 @@ export class AlertManager { detector: this.createDetector(id), watchingRingingCommand: null, outputSinceWatchingRing: false, - ringSeq: 0, episode: null, protocolStatus: 'IDLE', progress: null, @@ -1362,7 +1351,6 @@ function alertStatesEqual(a: AlertState, b: AlertState): boolean { || a.watchingEnabled !== b.watchingEnabled || a.todo !== b.todo || a.awaited !== b.awaited - || a.ringSeq !== b.ringSeq || a.episode?.id !== b.episode?.id ) return false; const an = a.notification; diff --git a/lib/src/lib/alert-resumed-output.test.ts b/lib/src/lib/alert-resumed-output.test.ts index 4ec2e6b32..a065c63fc 100644 --- a/lib/src/lib/alert-resumed-output.test.ts +++ b/lib/src/lib/alert-resumed-output.test.ts @@ -66,11 +66,12 @@ describe('WATCHING output resuming before alarm delivery', () => { it('cancels pending speech and push during animation, then gives the next settle a fresh delay', () => { busy(); settle(); - const firstRing = manager.getState(ID).ringSeq; + const firstEpisode = manager.getState(ID).episode; + expect(firstEpisode?.id).toBeTruthy(); expect(manager.getState(ID).status).toBe('ALERT_RINGING'); vi.advanceTimersByTime(1_000); busy(); - expect(manager.getState(ID)).toMatchObject({ status: 'BUSY', todo: false, ringSeq: firstRing }); + expect(manager.getState(ID)).toMatchObject({ status: 'BUSY', todo: false, episode: null }); // Keep animating across the old speech deadline, as in the marked incident. for (let i = 0; i < 50; i++) { @@ -81,7 +82,10 @@ describe('WATCHING output resuming before alarm delivery', () => { expect(pushed).not.toHaveBeenCalled(); settle(); - expect(manager.getState(ID)).toMatchObject({ status: 'ALERT_RINGING', ringSeq: firstRing + 1 }); + const relatched = manager.getState(ID); + expect(relatched.status).toBe('ALERT_RINGING'); + expect(relatched.episode?.id).toBeTruthy(); + expect(relatched.episode?.id).not.toBe(firstEpisode?.id); vi.advanceTimersByTime(DELAY - 1); expect(spoken).not.toHaveBeenCalled(); expect(pushed).not.toHaveBeenCalled(); @@ -138,9 +142,9 @@ describe('WATCHING output resuming before alarm delivery', () => { { type: 'commandStart', source: 'osc633_E', startedAt: Date.now() }, ]); } - const ringSeq = manager.getState(ID).ringSeq; + const episode = manager.getState(ID).episode; busy(); - expect(manager.getState(ID)).toMatchObject({ status: 'ALERT_RINGING', ringSeq }); + expect(manager.getState(ID)).toMatchObject({ status: 'ALERT_RINGING', episode }); vi.advanceTimersByTime(DELAY); expect(spoken).toHaveBeenCalledOnce(); }); diff --git a/lib/src/lib/session-save.test.ts b/lib/src/lib/session-save.test.ts index e2df85599..deed509c6 100644 --- a/lib/src/lib/session-save.test.ts +++ b/lib/src/lib/session-save.test.ts @@ -346,15 +346,16 @@ describe('saveSession', () => { it('persists local browser surface TODO state in the browser pane alert field', async () => { const platform = createPlatform(null); - // A full live ActivityState, so the assertion below shows the projection - // dropping the fields `docs/specs/alert.md` -> Public State forbids on disk. + // A full live ActivityState, episode included, so the assertion below shows + // the projection dropping every field `docs/specs/alert.md` -> Public State + // keeps off disk. terminalRegistryMocks.getActivity.mockReturnValue({ status: 'WATCHING_DISABLED', watchingEnabled: false, todo: true, notification: null, awaited: false, - ringSeq: 3, + episode: { id: 'episode-web', startedAt: 0 }, }); await saveSession(platform, [ diff --git a/lib/src/lib/session-types.ts b/lib/src/lib/session-types.ts index b4124a049..447c18611 100644 --- a/lib/src/lib/session-types.ts +++ b/lib/src/lib/session-types.ts @@ -45,10 +45,10 @@ export interface PersistedPane { } /** - * Narrow live Activity down to what may reach disk. An explicit projection, not - * a structurally-assignable pass-through: `ActivityState` is a superset, and - * `JSON.stringify` writes every extra field it grows - * (`docs/specs/alert.md` -> Public State, "Persist only"). + * Narrow Activity down to what may reach disk. The parameter deliberately uses + * the persisted shape: live `AlertState` is structurally assignable to it, and + * this explicit projection keeps `JSON.stringify` from writing extra live or + * stale fields (`docs/specs/alert.md` -> Public State, "Persist only"). */ export function toPersistedAlertState(state: PersistedAlertState): PersistedAlertState { return { diff --git a/lib/src/lib/terminal-registry.alert.test.ts b/lib/src/lib/terminal-registry.alert.test.ts index 4deb9140f..e261991c4 100644 --- a/lib/src/lib/terminal-registry.alert.test.ts +++ b/lib/src/lib/terminal-registry.alert.test.ts @@ -359,7 +359,7 @@ describe('terminal-registry alert behavior', () => { it('preserves pre-registration activity through terminal creation and orphaning', () => { const id = 'early-host-state'; - setTerminalActivity(id, { status: 'ALERT_RINGING', ringSeq: 7, todo: true, awaited: true }); + setTerminalActivity(id, { status: 'ALERT_RINGING', todo: true, awaited: true }); const activity = getActivity(id); expect(getLivePersistedAlertState(id)).toBeNull(); @@ -411,11 +411,11 @@ describe('terminal-registry alert behavior', () => { it('retains a resumed exited Session TODO until disposal', () => { const id = 'exited-host-state'; - setTerminalActivity(id, { todo: true, ringSeq: 3 }); + setTerminalActivity(id, { todo: true }); resumeTerminal(id, null, { alive: false, exitCode: 1 }); expect(registry.get(id)?.exited).toBe(true); - expect(getActivity(id)).toMatchObject({ todo: true, ringSeq: 3 }); + expect(getActivity(id)).toMatchObject({ todo: true }); expect(getLivePersistedAlertState(id)).toMatchObject({ todo: true }); disposeSession(id); diff --git a/lib/src/lib/workspace-union.test.ts b/lib/src/lib/workspace-union.test.ts index d71c7dd2e..75da6181b 100644 --- a/lib/src/lib/workspace-union.test.ts +++ b/lib/src/lib/workspace-union.test.ts @@ -9,7 +9,6 @@ function activity(entries: Record>): Map [id, { ...base, ...partial }])); } diff --git a/vscode-ext/test/session-state.test.ts b/vscode-ext/test/session-state.test.ts index f2ae9a487..7b0569131 100644 --- a/vscode-ext/test/session-state.test.ts +++ b/vscode-ext/test/session-state.test.ts @@ -52,7 +52,7 @@ describe('VS Code session alert persistence', () => { watchingEnabled: true, todo: true, awaited: true, - ringSeq: 7, + episode: { id: 'episode-live', startedAt: 0 }, })], ])) as PersistedSession; @@ -70,7 +70,7 @@ describe('VS Code session alert persistence', () => { notification: null, watchingEnabled: true, awaited: true, - ringSeq: 4, + episode: { id: 'episode-stale', startedAt: 0 }, }; const store = contextWithState({ version: 3, diff --git a/website/src/lib/tut-detector.test.ts b/website/src/lib/tut-detector.test.ts index de80ce532..1fa42dc99 100644 --- a/website/src/lib/tut-detector.test.ts +++ b/website/src/lib/tut-detector.test.ts @@ -9,7 +9,7 @@ function activity( todo = false, watchingEnabled = status !== "WATCHING_DISABLED", ): ActivityState { - return { status, watchingEnabled, todo, notification: null, awaited: false, ringSeq: 0 }; + return { status, watchingEnabled, todo, notification: null, awaited: false }; } function makeDetectorHarness(initialActivitySnapshot = new Map()) {