Defer alerts until terminal animation stops

[nedtwigg]

Summary

• add an off-by-default alert setting named Defer alerts until animation stops
• defer eligible terminal notifications while animation detection is fully armed, then emit one fresh protocol ring when the pane becomes quiet
• keep authoritative shell-reported command exits immediate; if one arrives with a notification pending, fold the richer protocol detail into the same summons
• coalesce bounded pending notification intent, cancel it on attendance or teardown, and keep speech and push downstream of the resulting ring
• document the alert lifecycle and add focused manager, detector, persistence, host-wiring, and UI coverage

Testing

• pnpm test
• pnpm build

[cloudflare-workers-and-pages]

Deploying mouseterm with    Cloudflare Pages

Latest commit:	785a646
Status:	✅  Deploy successful!
Preview URL:	https://3e08ced3.mouseterm.pages.dev
Branch Preview URL:	https://animation-can-suppress-spoke.mouseterm.pages.dev

View logs

[dormouse-bot]

One real bug: the defer branch swallows a publish the caller delegated to it, so a progress cycle that ends while output is still animating leaves subscribers pinned at OSC_NOTIF_BUSY. The rest of the change reads clean — the claimants-first ordering, the parallel quiet timer surviving detector.reset() at command boundaries, and the settings plumbing through all four adapters all check out.

finishProtocolProgressCycle clears the cycle before dispatch and hands the publish to the ring rules — its comment says so, and the spec restates it under Completion events -> "Two ordering rules": "Clear the progress cycle before dispatch, so a completion or error ends the cycle whether or not the event is claimed and OSC_NOTIF_BUSY falls back either way." Before this PR setProtocolRinging ended with this.notify(id), which is what made that true on the unclaimed path. deferOrDeliverHumanAlert's defer branch returns without it, so with deferAlertsUntilQuiet on, the fallback happens internally but nobody is told.

Repro I ran against the merged tree — fails on this branch, passes with the setting off:

it('publishes the OSC_NOTIF_BUSY fallback when a progress cycle completes under deferral', () => {
  const id = 'progress-defer';
  const seen: string[] = [];
  manager.onStateChange((_id, s) => seen.push(s.status));
  manager.setDeferAlertsUntilQuiet(true);
  driveToBusy(id);

  manager.updateProtocolProgress(id, { state: 'normal', percent: 40 });
  expect(manager.getState(id).status).toBe('OSC_NOTIF_BUSY');
  seen.length = 0;

  manager.updateProtocolProgress(id, { state: 'normal', percent: 100 });

  expect(manager.getState(id).status).not.toBe('OSC_NOTIF_BUSY'); // passes
  expect(seen).not.toEqual([]);                                   // fails
});

The stale projection persists until something else calls notify — for an unwatched command under continuous output, that can be a long time, since onData doesn't notify and the detector's onChange only does while WATCHING. The commandFinished path is fine by luck: finishCommandExitWatch returns truthy and applyTerminalSemanticEvents / onExit notify for it.

The inline suggestion is the one-line fix; I ran it against the full alert suite (alert-manager, quiesce-detector, alert-settings, alert-settings-host) and all 138 tests pass, including the nine new deferral cases. Worth adding the repro above as a test — the existing deferral cases all go through notifyFromProtocol, which is why this path had no coverage.

Two smaller things:

• The second inline comment is a spec-convention fix. AGENTS.md: "Rationale entries are keyed by the spec's own headings ... the pairing is by heading." The (rationale) marker sits on the last deferral bullet under ## Completion events, but the three new entries are filed under ## Alarm settings, so a reader following the marker by heading finds nothing. spec-lint passes because ## Alarm settings does exist in the spec. If you'd rather keep "Why animation deferral defaults off" keyed to where the default is documented, splitting it into two headings works equally well.
• clearTodo's new if (clearedDeferred) this.notify(id) is a no-op once the above is fixed: deferred intent isn't part of AlertState, nothing else in that branch mutates the entry, and notify dedupes against lastEmitted. Today it fires only as an accidental side effect of the swallowed publish above.

[nedtwigg]

Addressed the full dormouse-bot review in bd007d2: fixed the swallowed OSC_NOTIF_BUSY fallback publication, added the exact regression case, realigned rationale headings/markers, and removed the redundant clearTodo notification. Focused alert/settings suite: 138 tests passed; spec lint and diff check pass.

[dormouse-bot]

Verified bd007d2 — all three findings resolved, and my approval stands.

The defer branch now publishes after recording the pending alert, so the delegated OSC_NOTIF_BUSY fallback reaches subscribers, and the regression test pins exactly the path that had no coverage. The rationale entries for the detector gate and the no-redispatch rule now sit under ## Completion events where their marker points, with deferAlertsUntilQuiet carrying its own marker for the default-off entry under ## Alarm settings. Dropping the clearTodo notify is right — it was only ever firing off the swallowed publish.

Nothing further from me. The only non-terminal check is Chromatic UI Tests (19 changes awaiting baseline acceptance), which needs you in the Chromatic UI; every automated check is green at bd007d2.

[dormouse-bot]

The deferral reshape traces clean. I walked the four paths the two new commits touch — command finish with and without a pending notification, the notification carried across a command-boundary reset, the self-re-arming scheduleDeferredNotification, and every caller of clearAllRingsIfActive — and the state machine holds. Specifically: the re-arm cannot spin (quietAt() only advances on accepted output, and each advance is bounded by one quiet window); flushDeferredNotification's hasAttention early return is genuinely unreachable-by-construction as its comment claims, since a deferral can only be created while unattended and attend() cancels it; and clearDeferredNotification is reached on every path that previously called it, with clearTodo's explicit call correctly kept for the early return. Build & Test is green at 77c1d87.

Two documentation points, no code concerns.

The spec's command-exit pointer names a symbol this PR renamed away. docs/specs/alert.md -> Command-exit Track still reads "stores the COMMAND_EXIT notification built by setCommandExitRinging / formatCommandExitBody", but the method is applyCommandExitRinging now — formatCommandExitBody still resolves, so it's just the one name. It's outside the diff hunks so I can't suggest it inline; spec-lint doesn't catch it either, since it checks backticked repo paths, not symbols (setCommandExitRinging and T_USER_ATTENTION are the only two backticked identifiers in alert.md that no longer resolve in lib/, and the latter is a spec-level timer name by convention). Happy to push the one-word fix if you'd like.

The PR description is now stale. It says the change defers "eligible terminal-report and command-finished rings", which 77c1d87 deliberately undid. Worth a pass before merge if this squashes into the commit message.

[dormouse-bot]

The applySettings doc three lines up says "the sinks below revalidate anyway", but this commit dropped typeof enabled !== 'boolean' from this sink, so only setInactivityTimeoutMs still does. Every live caller normalizes first (AlertSettingsHost.apply for VS Code, alert-settings.ts for the in-process adapters), so nothing is broken today — but the pair is now asymmetric and the new comment overclaims. Restoring the clause is the smaller reconciliation; dropping the claim works equally well.

Suggested change

	if (enabled === this.deferAlertsUntilQuiet) return;
	if (typeof enabled !== 'boolean' || enabled === this.deferAlertsUntilQuiet) return;

[nedtwigg]

Narrowed the setting after product review in 77c1d87: animation deferral now applies only to terminal notifications. Shell-reported command exits ring immediately; if one arrives while a notification is pending, the richer protocol detail folds into that same immediate summons. Updated settings copy/spec and added regression coverage for immediate exits, mixed pending notification + exit, and notification survival across an unarmed command-boundary reset. Full pnpm test and pnpm build pass.

[nedtwigg]

Addressed both documentation findings: 785a646 updates the stale command-exit source symbol to applyCommandExitRinging, and the PR description now states that only terminal notifications defer while shell-reported exits remain immediate.

[dormouse-bot]

Verified 785a646 — both documentation points are resolved and my approval stands.

The spec pointer now names applyCommandExitRinging, which resolves at lib/src/lib/alert-manager.ts alongside formatCommandExitBody; no setCommandExitRinging references remain in docs/specs/alert.md. The PR description now scopes deferral to terminal notifications and states the immediate-exit rule, matching what 77c1d87 actually does.

Nothing further from me. Checks are still running at 785a646; I'll dismiss the approval if anything comes back red.