Add "Play test sound" and "Send test push" to the alarm settings - #424
Add "Play test sound" and "Send test push" to the alarm settings#424nedtwigg wants to merge 1 commit into
Conversation
An alarm you cannot observe until it fires unattended is one you cannot trust, and that first firing is exactly when being wrong costs the most. Give each alarm sink a way to try it now. Both sit outside the switch's dimming and stay enabled while the sink is off: checking that the speakers work, or that the phone buzzes, is most useful before committing to the alarm. Each reports its outcome inline, because for both sinks a working path and a broken one produce the same observation — silence. Play test sound deliberately does not route through `speak()`: that publishes the transient per-Session speaking/spoken state that Panes and Doors render, and no Session rang. A test that lit up a pane would be claiming some terminal wants attention. It reports a webview with no speech backend rather than degrading silently the way the alarm path correctly does. Send test push goes through the real Host, ACL and server, so what it proves is what the alarm will do. That needed the delivery path to become observable: `sendPush` now returns a summary instead of `void`, and a new `pushTest` service command surfaces failures rather than warning to the console. The ring path's rule that a failed push must never break the alert path is right for an alarm and exactly wrong for a test — it would report success over a fan-out that reached nobody. Four outcomes are distinguished: nothing targeted (the ordinary answer on a freshly enrolled machine, not a failure), nothing delivered, a partial fan-out, and success. Note the signature change: `sendPush` returns `Promise<PushSendSummary>` rather than `Promise<void>`. The ring path ignores the value, so behavior is unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GDNJHHA95nvRdo4Cv3rAoi
Deploying mouseterm with
|
| Latest commit: |
8a41810
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://cf1d60fa.mouseterm.pages.dev |
| Branch Preview URL: | https://alarm-test-buttons.mouseterm.pages.dev |
dormouse-bot
left a comment
dormouse-bot
left a comment
There was a problem hiding this comment.
Nice feature, and the framing is right — routing the push test through the real Host, ACL and server rather than a mock is what makes it worth having, and #pushTest being the deliberate inverse of #push on failure-swallowing is the correct call. Three notes below; only the first is a bug.
Outside the diff: alert-speech.ts's module docstring still claims that "speak() and cancelSpeech() are the only two places this module touches the engine — the seam a future native PlatformAdapter.speak?() would slot into for hosts whose webview has no speech backend (Tauri on Linux/WebKitGTK)". speakTestUtterance is now a third, calling synth.cancel() and synth.speak() directly. That seam now has three sites, and a native-speech implementation written against that docstring would leave the test button broken on exactly the host it exists to diagnose. I can push a commit fixing the docstring if you'd like it.
Also minor: the spec says the button "distinguishes four outcomes", and AlarmTestButtons.test.tsx covers three — the partial fan-out branch (Sent to N; M failed.) is the one with no test.
| try { | ||
| // Drop anything queued first: repeated presses should say it once more, not | ||
| // stack a backlog behind a slow engine. | ||
| synth.cancel(); |
There was a problem hiding this comment.
cancel() empties the whole engine queue, not just this function's own utterance, and startAlertSpeech has no way to learn it happened.
Concretely: Pane A is mid-announcement with Pane B's utterance queued behind it, and the user presses Play test sound. Web Speech fires end only for the utterance actually being spoken, so A settles normally but B's is dropped with no callback — startAlertSpeech leaves B sitting in its queued map and never re-dispatches it, so B's alarm is never announced. That is exactly the case interrupt() was written for ("attending one Pane must not silence another Pane's alarm"), and the reason it re-dispatches by hand is stated right there: cancel() is not obliged to fire a callback per dropped utterance.
Narrow window — the user has to be in Settings while another Pane is mid-alarm — but the failure is a real alarm going out silently, which is the costliest thing this subsystem can do. Two ways out: drop the cancel() (two short fixed phrases stacking on a double-press is a smaller problem than a dropped alarm), or give startAlertSpeech a way to hand interrupt() out so the test button can go through it rather than around it.
| * than an error. Anything the user could act on differently deserves its own | ||
| * answer, and "no devices" and "the server refused" are not the same problem. | ||
| */ | ||
| export interface PushTestOutcome { |
There was a problem hiding this comment.
PushTestOutcome is structurally identical to PushSendSummary in push-delivery.ts, and sendTestPush bridges them with an unchecked as PushTestOutcome — so the two can drift with no type error.
service-protocol.ts is the module that exists to prevent precisely this ("Shared by both ends so the contract cannot drift") and already owns PushDevicesResult, AdoptResult and EnrollResult. pushTest's result looks like it belongs there too, with push-delivery.ts importing it instead of declaring a parallel copy.
As written, adding a field to PushSendSummary leaves PushTestOutcome stale and the cast swallows it — the button reads undefined and nothing fails to compile.
| function ResultLine({ result }: { result: { text: string; tone: 'ok' | 'bad' } | null }) { | ||
| if (!result) return null; | ||
| return ( | ||
| <div className={`mt-1 text-sm leading-relaxed ${result.tone === 'bad' ? 'text-error' : 'text-muted'}`}> |
There was a problem hiding this comment.
The one thing this control exists to communicate is never announced to a screen reader — the result line just appears. The alert layer's own AlertSpeechIndicator.tsx uses role="status" + aria-live="polite" for the same job.
| <div className={`mt-1 text-sm leading-relaxed ${result.tone === 'bad' ? 'text-error' : 'text-muted'}`}> | |
| <div | |
| role="status" | |
| aria-live="polite" | |
| className={`mt-1 text-sm leading-relaxed ${result.tone === 'bad' ? 'text-error' : 'text-muted'}`} | |
| > |
2 participants
An alarm you cannot observe until it fires unattended is one you cannot trust, and that first firing is exactly when being wrong costs the most. Each alarm sink gets a way to try it now.
Both controls sit outside the switch's dimming and stay enabled while the sink is off: checking that the speakers work, or that the phone buzzes, is most useful before committing to the alarm. Each reports its outcome inline and clears it after a few seconds — because for both sinks, a working path and a broken one produce the same observation: silence.
Play test sound
Speaks a fixed phrase through the same sanitizer a real alarm uses, but deliberately not through
speak(). That publishes the transient per-Sessionspeaking/spokenstate that Panes and Doors render, and no Session rang — a test that lit up a pane would be claiming some terminal wants attention.It reports "no speech engine available" rather than degrading silently the way the alarm path correctly does, because a webview with no backend and one with the volume down are indistinguishable otherwise.
Send test push
Goes through the real Host, ACL, and server, so what it proves is what the alarm will do.
That required making the delivery path observable.
sendPushnow returns a summary instead ofvoid, and a newpushTestservice command surfaces failures rather than warning to the console. The ring path's rule that a failed push must never break the alert path is right for an alarm and exactly wrong for a test — it would report success over a fan-out that reached nobody.Four outcomes are distinguished, because they call for different responses:
targeted: 0delivered: 0Hidden entirely where no Host service exists, matching the Remote control section.
API change worth flagging
sendPushreturnsPromise<PushSendSummary>rather thanPromise<void>. The ring path ignores the value, so behavior there is unchanged — but it is a signature change.Verification
typecheck clean; 1689 tests passing;
pnpm lint:specsOK. 11 new tests: 4 at the service level (not-connected, nothing-targeted, a real send asserting ACL-derived recipients and the fixed collapse tag, and a refused send), 7 at the component level covering every outcome above plus the missing-speech-engine and no-Host-service cases.alert.mdupdated with the behavior and the rationale for each rule.🤖 Generated with Claude Code
https://claude.ai/code/session_01GDNJHHA95nvRdo4Cv3rAoi