Conversation
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 <noreply@anthropic.com>
…ynthetic 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 <noreply@anthropic.com>
Deploying mouseterm with
|
| Latest commit: |
a8037a4
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://7b1baad7.mouseterm.pages.dev |
| Branch Preview URL: | https://alert-dismiss-simplify.mouseterm.pages.dev |
dormouse-bot
left a comment
There was a problem hiding this comment.
Feedback on work in progress, not a merge verdict — mark the PR ready when you want the full review.
The collapse to one action reads well, and the PR's claim about the watched-command list checks out: WatchedCommandList is mounted only by SettingsDialog, so the spec line and both comments really were wrong.
One consequence worth weighing. The alert button and a now call dismissSessionAlert unconditionally, so AlertManager.dismissAlert is reached in states the old transition table never routed to it — and clearAllRingsIfActive cancels a pending deferred notification before it reports that nothing was ringing. With deferral now the default from the base PR (#706), a pane whose agent emitted its "done" notification while the TUI is still redrawing is holding that notification behind deferredNotificationTimer with nothing latched. Clicking the alert button there drops it outright: no ring, no TODO, no notification detail, and dismissAlert returns before notify, so nothing re-renders either. The button's own new copy — Alert settings, "Click or right-click for options" — invites exactly that click, while right-click and > open the same context without the loss, so the two routes the spec presents as interchangeable aren't.
It also makes the new rule "Dismissing a Session with nothing ringing changes nothing and notifies no one" false as written; the test of that name dismisses a notification that rang immediately, so the deferred case is uncovered. The inline suggestion gates on hasActiveRing first, which restores the rule literally and leaves attend's eager cancellation alone.
Dismissing asked whether a ring was active by calling the clear-everything helper, which cancels the deferred-notification timer on its way through. With deferral on by default, clicking the alert button on a pane whose agent had already reported while its TUI was still redrawing destroyed that notification and returned before notifying anyone. Ask `hasActiveRing` first, so a Session with nothing ringing is a real no-op. Also drop `actions` from the alert button callback's deps; it no longer reads it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
# Conflicts: # docs/specs/alert.md
dormouse-bot
left a comment
There was a problem hiding this comment.
Feedback on work in progress, not a merge verdict — mark the PR ready when you want the full review.
The gate lands where the last round suggested, and the new test pins it from the right side. One thing the commit leaves behind.
docs/specs/alert.md, under Completion events, still says of the deferral: Cancel pending delivery on attendance, dismissal, TODO changes, removal, seeding, or teardown. Dismissal is now the one entry in that list that doesn't — a dismiss on a quiet Session is exactly the case hasActiveRing carves out, and it is the behavior the new test asserts. The rest of the list still holds: attend, toggleTodo, markTodo, and clearTodo all call clearAllRingsIfActive unconditionally. AGENTS.md → "Specs": "When code covered by a spec changes, change the spec."
The line is outside this PR's hunks, so no inline suggestion — something like:
- Cancel pending delivery on attendance, a dismissal that clears a ring, TODO changes, removal, seeding, or teardown. A dismiss on a quiet Session keeps it: a cancelled deferral was never visible to dismiss. Disabling the setting releases it immediately; …
alert.md sits at 7,174 words against the 7,200 you ratcheted to, so ~26 words of headroom before it needs another --ratchet; that rewrite spends about 19.
|
The follow-up spec finding is fixed in a8037a4. Completion events now say that only a dismissal which clears a visible ring cancels pending delivery, while a quiet-session dismiss preserves a notification that was never visible to dismiss. spec-lint passes. |
|
Confirmed at a8037a4 — the Completion-events line now reads Cancel pending delivery on attendance, a dismissal that clears a ring, TODO changes, removal, seeding, or teardown, which matches the Nothing outstanding from my side. |
Stacked on #706.
The pane-header alert button,
ain command mode, and the mobile header button used to run a five-way transition table: dismiss, or consumeattentionDismissedRing, or toggle the running command's WATCHING rule, or open the context. Now they do one thing: dismiss a ringing Session (setting TODO, as before) and open the terminal context. WATCHING rules are created only in the terminal context ("Watch all<cmd>commands") and removed there or in Settings.attentionDismissedRing,dismissOrToggleAlert,AlertButtonActionResult,toggleSessionAlert,disableSessionAlertdeleted;ActivityStateis plainAlertState.aand>open the context through a Wall-providedopenTerminalContextinstead of clicking the bell or faking acontextmenuevent.alert.mdbudget ratcheted 7250 → 7200.Next PR in the series makes every ring flash the Pane; the one after retires the bell icon.
🤖 Generated with Claude Code