diff --git a/__tests__/dismissalReplay.test.ts b/__tests__/dismissalReplay.test.ts index f7091b9..70dcc4b 100644 --- a/__tests__/dismissalReplay.test.ts +++ b/__tests__/dismissalReplay.test.ts @@ -56,11 +56,14 @@ describe('dismissal memory', () => { expect(d.suppress('a', undefined, false)).toBe(true); }); - it('lets a live socket frame clear the memory — a new trigger must ring', () => { + it('does not let a live socket frame undo a dismissal', () => { + // Measured: after dismissing, a queued native takeover replayed the trigger + // down the socket and the siren came back. The alert is triggered once, so + // a second "live" frame for a dismissed alert is the same alarm. const d = createDismissals(); d.mark('a', EVENT); - expect(d.suppress('a', EVENT, true)).toBe(false); - expect(d.suppress('a', EVENT, false)).toBe(false); + expect(d.suppress('a', EVENT, true)).toBe(true); + expect(d.suppress('a', EVENT, false)).toBe(true); }); it('seeds from the durable ack queue so a replay before the flush is silent', () => { @@ -74,6 +77,65 @@ describe('dismissal memory', () => { }); }); +describe('a dismissal must survive the app being killed', () => { + it('queues the ack even before the device identity is loaded', () => { + // Measured against the service: a device-token ack with no deviceId is + // accepted (the credential identifies the subscription). `acknowledgeAlarm` + // used to `return` when `deviceId` was still null, silently dropping the + // dismissal — the alarm then owed the ledger again and rang on reopen. + const context = fs.readFileSync( + path.join(__dirname, '..', 'src', 'state', 'AlarmContext.tsx'), + 'utf8', + ); + const at = context.indexOf('const acknowledgeAlarm = useCallback'); + const body = context.slice(at, at + 1600); + expect(body).not.toContain('if (!deviceId) return;'); + expect(body).toContain('enqueueAck('); + expect(body).toContain('deviceId: deviceId ?? undefined'); + }); + + it('does not raise an alarm before the queued dismissals are known', () => { + const context = fs.readFileSync( + path.join(__dirname, '..', 'src', 'state', 'AlarmContext.tsx'), + 'utf8', + ); + const at = context.indexOf('const handleTrigger = useCallback'); + const body = context.slice(at, at + 900); + expect(body).toContain('dismissalsReadyRef.current'); + // …and the armed service must not start listening before that either. + const armAt = context.indexOf('const armedAttemptRef'); + const arm = context.slice(armAt, armAt + 900); + expect(arm).toContain('if (!dismissalsReady) return;'); + }); + + it('unsubscribes the device as part of acknowledging', () => { + // Owner ruling: a subscriber that acknowledged is done with the alarm, so + // the acknowledgement drops the subscription instead of leaving a live + // address that rings again on the next launch. + const context = fs.readFileSync( + path.join(__dirname, '..', 'src', 'state', 'AlarmContext.tsx'), + 'utf8', + ); + const at = context.indexOf('const acknowledgeAlarm = useCallback'); + const body = context.slice(at, at + 2600); + expect(body).toContain('api.removeSubscription('); + expect(body).toContain("markEnded(alertId, 'left')"); + // The ack settles the ledger *before* the row goes, so a failed removal can + // never leave the debt behind. + expect(body.indexOf('flushAcks(')).toBeLessThan(body.indexOf('api.removeSubscription(')); + }); + + it('flushes a queued ack that has no device id of its own', () => { + const source = fs.readFileSync( + path.join(__dirname, '..', 'src', 'services', 'pendingAcks.ts'), + 'utf8', + ); + expect(source).toContain('deviceId?: string;'); + expect(source).toContain('flushAcks(fallbackDeviceId?: string)'); + expect(source).toContain('ack.deviceId ?? fallbackDeviceId'); + }); +}); + describe('dismissal reaches the service', () => { const screen = fs.readFileSync( path.join(__dirname, '..', 'src', 'screens', 'LiveAlarmScreen.tsx'), diff --git a/__tests__/nativeDelivery.test.ts b/__tests__/nativeDelivery.test.ts index 136e8e1..889f733 100644 --- a/__tests__/nativeDelivery.test.ts +++ b/__tests__/nativeDelivery.test.ts @@ -39,10 +39,41 @@ describe('native alarm delivery contract', () => { it('schedules a real alarm clock with AlarmManager.setAlarmClock', () => { expect(scheduler).toContain('setAlarmClock('); expect(scheduler).toContain('AlarmClockInfo('); - expect(scheduler).toContain('AlarmActivity.intent('); expect(scheduler).toContain('canScheduleExactAlarms'); }); + it('never makes the alarm clock entry launch an activity', () => { + // Measured on Android 15: a background activity launch from the alarm + // clock's PendingIntent is refused (BAL_BLOCK) and then *deferred*, so the + // window surfaced later — after the alarm had been acknowledged — and the + // siren came back for a delivered alert. The screen is raised by the + // full-screen-intent notification; the entry itself must only wake the + // process. + const fireAt = scheduler.indexOf('fun fireNow('); + const fire = scheduler + .slice(fireAt, fireAt + 2000) + .split('\n') + .filter(line => !line.trimStart().startsWith('//')) + .join('\n'); + expect(fire).toContain('PendingIntent.getBroadcast('); + expect(fire).toContain('AlarmWakeReceiver::class.java'); + expect(fire).not.toContain('PendingIntent.getActivity('); + expect(fire).not.toContain('AlarmActivity.intent('); + // The one activity PendingIntent left is the alarm-clock *show* intent, + // which only the user can invoke by tapping the status-bar alarm icon. + const showAt = scheduler.indexOf('private fun showIntent('); + const show = scheduler.slice(showAt); + expect(show).toContain('PendingIntent.getActivity('); + const code = scheduler + .split('\n') + .filter(line => !line.trimStart().startsWith('//')) + .join('\n'); + expect(code.split('PendingIntent.getActivity(').length - 1).toBe(1); + // The receiver must ignore an entry that outlived the arming. + expect(receiver).toContain('AlarmScheduler.ACTION_FIRE'); + expect(receiver).toContain('armed?.alertId != alertId'); + }); + it('fires the alarm from the trigger frame through the scheduler, not startActivity', () => { expect(service).toContain('AlarmScheduler.fireNow('); expect(service).not.toMatch(/startActivity\(AlarmActivity\.intent/); diff --git a/android/app/src/main/java/com/dcntrlized/alarm/AlarmScheduler.kt b/android/app/src/main/java/com/dcntrlized/alarm/AlarmScheduler.kt index 2a512aa..f783736 100644 --- a/android/app/src/main/java/com/dcntrlized/alarm/AlarmScheduler.kt +++ b/android/app/src/main/java/com/dcntrlized/alarm/AlarmScheduler.kt @@ -70,7 +70,12 @@ object AlarmScheduler { private const val REQ_SHOW = 4203 const val ACTION_STANDING = "com.dcntrlized.alarm.STANDING" + + /** The entry that ends the ring: the alarm clock's own operation. */ + const val ACTION_FIRE = "com.dcntrlized.alarm.FIRE" + const val EXTRA_ALERT_ID = "alarm.scheduler.alertId" + const val EXTRA_MESSAGE = "alarm.scheduler.message" /** * `USE_EXACT_ALARM` (declared in the manifest) makes this true without user @@ -95,10 +100,28 @@ object AlarmScheduler { } val am = context.getSystemService(Context.ALARM_SERVICE) as? AlarmManager ?: return false return try { - val operation = PendingIntent.getActivity( + // The operation is a broadcast, NEVER an activity launch. + // + // This used to be `PendingIntent.getActivity(AlarmActivity)`, and on + // Android 14+ that launch is refused from the background + // (`BAL_BLOCK`). Worse than being dropped, it is *deferred*: the + // activity sits in the task and surfaces later — measured after an + // acknowledgement, where pressing back let the queued takeover come + // to the front again, re-playing the siren for an alert that had + // already been delivered (and therefore rendering a title-less alarm + // that fell back to the raw alert id). + // + // The screen is raised by the full-screen-intent notification, which + // SystemUI launches; the alarm clock only has to make the platform + // treat this as an alarm and wake the process at the right time. + val operation = PendingIntent.getBroadcast( context, REQ_FIRE, - AlarmActivity.intent(context, alertId, message), + Intent(context, AlarmWakeReceiver::class.java).apply { + action = ACTION_FIRE + putExtra(EXTRA_ALERT_ID, alertId) + putExtra(EXTRA_MESSAGE, message) + }, PendingIntent.FLAG_UPDATE_CURRENT or PendingIntent.FLAG_IMMUTABLE ) am.setAlarmClock( @@ -176,10 +199,10 @@ object AlarmScheduler { val am = context.getSystemService(Context.ALARM_SERVICE) as? AlarmManager ?: return try { am.cancel( - PendingIntent.getActivity( + PendingIntent.getBroadcast( context, REQ_FIRE, - AlarmActivity.intent(context, "", ""), + Intent(context, AlarmWakeReceiver::class.java).apply { action = ACTION_FIRE }, PendingIntent.FLAG_UPDATE_CURRENT or PendingIntent.FLAG_IMMUTABLE ) ) diff --git a/android/app/src/main/java/com/dcntrlized/alarm/AlarmWakeReceiver.kt b/android/app/src/main/java/com/dcntrlized/alarm/AlarmWakeReceiver.kt index 6af96c4..2e6e692 100644 --- a/android/app/src/main/java/com/dcntrlized/alarm/AlarmWakeReceiver.kt +++ b/android/app/src/main/java/com/dcntrlized/alarm/AlarmWakeReceiver.kt @@ -33,6 +33,29 @@ class AlarmWakeReceiver : BroadcastReceiver() { Log.i(TAG, "boot: standing entry re-armed=$ok for ${armed.alertId}") } + AlarmScheduler.ACTION_FIRE -> { + // The ring's own alarm-clock entry. It must NOT start an + // activity (that is what the full-screen-intent notification is + // for, and a background activity launch is refused and then + // deferred by the platform, which is how a dismissed alarm came + // back). All this has to do is make sure the process is awake + // and the takeover is on screen. + val alertId = intent.getStringExtra(AlarmScheduler.EXTRA_ALERT_ID) + val message = intent.getStringExtra(AlarmScheduler.EXTRA_MESSAGE) ?: "" + val armed = AlarmArmStore.current(context) + if (alertId.isNullOrEmpty() || armed?.alertId != alertId) { + // Dismissed, disarmed or replaced between scheduling and + // firing: there is nothing to ring. + Log.i(TAG, "fire ignored: not armed for $alertId") + return + } + Log.i(TAG, "fire for $alertId") + AlarmNotifier.showFiring(context, alertId, message) + if (!AlarmForegroundService.isRunning) { + AlarmForegroundService.start(context, armed.alertId, armed.wsUrl) + } + } + AlarmScheduler.ACTION_STANDING -> { val armed = AlarmArmStore.current(context) ?: return // Chain the next wake before doing anything that could fail. diff --git a/src/lib/dismissals.ts b/src/lib/dismissals.ts index 988f73c..96ba88a 100644 --- a/src/lib/dismissals.ts +++ b/src/lib/dismissals.ts @@ -11,10 +11,14 @@ * - a dismissal that could not name one (the native takeover launch carries * only the alert) is recorded as `*`: it suppresses the trigger current at * dismissal time, and is pinned to that concrete event as soon as a status - * report reveals it, so a later trigger is not swallowed; - * - a live socket frame is a genuinely new trigger from the owner (replays of - * acknowledged deliveries never reach the client), so it clears the memory - * for that alert instead of being suppressed. + * report reveals it; + * - a dismissal is **not** undone by the transport. It used to be: a live + * socket frame cleared the memory on the theory that a live frame must be a + * new trigger. That is wrong on this service — an alert is triggered once + * and then disarmed or delivered, so a live frame for a dismissed alert is + * the same alarm arriving down another pipe (measured: after dismissing, the + * deferred native takeover replayed a frame and the siren came back). The + * only thing that clears the memory is the alert ending (`forget`). * * Pure and in-memory on purpose: the durable part of a dismissal is the ack * queue, and correctness must not depend on storage having finished writing. @@ -31,8 +35,9 @@ export interface DismissalMemory { /** * Should a trigger raise be suppressed? * - * `live` frames are new triggers and clear the memory; status/check-in - * raises are suppressed when this device already dismissed that event. + * Suppressed when this device dismissed the alert — whether the raise came + * from a status report or the socket (`live` only changes where the frame + * came from, never whether the user already dismissed it). */ suppress(alertId: string, eventId: string | undefined, live: boolean): boolean; forget(alertId: string): void; @@ -53,14 +58,14 @@ export function createDismissals(): DismissalMemory { seen.delete(alertId); }; - const suppress = (alertId: string, eventId: string | undefined, live: boolean) => { - if (live) { - forget(alertId); - return false; - } + // `live` is accepted for call-site symmetry and deliberately ignored: the + // transport a frame arrived on says nothing about whether a human dismissed + // the alarm (see the header comment). + const suppress = (alertId: string, eventId: string | undefined, _live: boolean) => { const set = seen.get(alertId); if (!set) return false; - if (eventId && set.has(eventId)) return true; + if (!eventId) return true; + if (set.has(eventId)) return true; if (!set.has('*')) return false; // Pin the anonymous dismissal to the event it actually covered; a later // trigger carries a different id and must ring. diff --git a/src/screens/AlertDetailScreen.tsx b/src/screens/AlertDetailScreen.tsx index 8532b23..26842a1 100644 --- a/src/screens/AlertDetailScreen.tsx +++ b/src/screens/AlertDetailScreen.tsx @@ -103,6 +103,10 @@ const ENDING_COPY: Record = { title: 'This alert was deleted', body: 'The alert and its tokens are gone.', }, + left: { + title: 'You left this alert', + body: 'This device acknowledged the alarm and unsubscribed, so it will not ring here again. The alert may still be live for everyone else.', + }, }; export default function AlertDetailScreen() { diff --git a/src/screens/HomeScreen.tsx b/src/screens/HomeScreen.tsx index 84397d2..6d455ac 100644 --- a/src/screens/HomeScreen.tsx +++ b/src/screens/HomeScreen.tsx @@ -64,6 +64,7 @@ const ENDING_COPY: Record['lastEnding']> expired: 'Expired — its lifetime ran out.', disarmed: 'Ended — you disarmed it.', deleted: 'Deleted.', + left: 'Acknowledged — this device unsubscribed.', }; export default function HomeScreen() { diff --git a/src/screens/LiveAlarmScreen.tsx b/src/screens/LiveAlarmScreen.tsx index cb6d425..1d1eb4c 100644 --- a/src/screens/LiveAlarmScreen.tsx +++ b/src/screens/LiveAlarmScreen.tsx @@ -161,10 +161,23 @@ export default function LiveAlarmScreen() { }; }, [fade, shake, reduceMotion]); + /** + * Is there an alarm to sound at all? + * + * A route can be opened for an alert that no longer exists: the native + * launch carries only an id, and on the way out of an alarm the platform can + * still hand the window a stale intent. Ringing for it is the "siren comes + * back after dismissing, showing the alert id" failure, so playback waits for + * evidence that the alarm is live — either the alert this device follows, or + * a trigger event that actually arrived. + */ + const live = !!alert || event?.action === 'trigger'; + // Native takeover: max-volume STREAM_ALARM playback + vibration. When the OS // refuses the alarm stream, the only thing that can still reach the user is // the volume warning — the screen says so instead of pretending. useEffect(() => { + if (!live) return; let cancelled = false; (async () => { const res = await playAlarm(alertId, message ?? ''); @@ -176,7 +189,7 @@ export default function LiveAlarmScreen() { cancelled = true; stopAlarm(); }; - }, [alertId, message]); + }, [alertId, message, live]); const leave = useCallback(() => { clearLiveEvent(); @@ -277,7 +290,7 @@ export default function LiveAlarmScreen() { {/* The alert name is the headline: at the moment of maximum stress this is the one piece of information the user needs (D-5). */} - {alert?.name ?? alertId} + {alert?.name ?? 'Alarm'} {attribution} diff --git a/src/services/pendingAcks.ts b/src/services/pendingAcks.ts index a170521..cab1998 100644 --- a/src/services/pendingAcks.ts +++ b/src/services/pendingAcks.ts @@ -25,7 +25,13 @@ export interface PendingAck { * could not be named would otherwise replay forever. */ eventId?: string; - deviceId: string; + /** + * The caller's device id. Absent when the dismissal happened before the + * identity was loaded — the service accepts a device-token ack without it + * (measured), and dropping the entry instead is how a dismissed alarm came + * back after the app was closed. + */ + deviceId?: string; /** Subscriber credential for the alert (rev 10); absent for accounts. */ deviceToken?: string; } @@ -50,7 +56,8 @@ function parse(raw: string | null): PendingAck[] { typeof (v as PendingAck).alertId === 'string' && ((v as PendingAck).eventId === undefined || typeof (v as PendingAck).eventId === 'string') && - typeof (v as PendingAck).deviceId === 'string' && + ((v as PendingAck).deviceId === undefined || + typeof (v as PendingAck).deviceId === 'string') && ((v as PendingAck).deviceToken === undefined || typeof (v as PendingAck).deviceToken === 'string'), ); @@ -88,11 +95,11 @@ export async function dropAck(alertId: string, eventId?: string): Promise * Send every queued ack. Stops on the first transport/server failure (the queue * is retried later); drops entries on success and on 404 ("settled"). */ -export async function flushAcks(): Promise { +export async function flushAcks(fallbackDeviceId?: string): Promise { const list = await loadAcks(); for (const ack of list) { try { - await ackAlarm(ack); + await ackAlarm({ ...ack, deviceId: ack.deviceId ?? fallbackDeviceId }); } catch (e) { // 404: the alert is gone. 401 for a device token: the alert was destroyed // by the very ack that preceded this retry, so the credential no longer diff --git a/src/state/AlarmContext.tsx b/src/state/AlarmContext.tsx index bed9b78..696206c 100644 --- a/src/state/AlarmContext.tsx +++ b/src/state/AlarmContext.tsx @@ -26,7 +26,13 @@ const ADMIN_ALERTS_KEY = 'alarm.adminAlerts.v1'; const SUBSCRIBED_ALERTS_KEY = 'alarm.subscribedAlerts.v1'; /** Why an alert no longer exists. Each ending has its own explicit copy. */ -export type EndingReason = 'delivered' | 'expired' | 'disarmed' | 'deleted'; +export type EndingReason = + | 'delivered' + | 'expired' + | 'disarmed' + | 'deleted' + /** This device acknowledged and left the alert (a dismissal *is* leaving). */ + | 'left'; export interface LiveAlarmEvent { alertId: string; @@ -138,6 +144,15 @@ export function AlarmProvider({ children }: { children: React.ReactNode }) { * dismissal time and is replaced by the concrete id as soon as one is known. */ const dismissals = useRef(createDismissals()); + /** + * Whether the durable queue has been read into [dismissals]. + * + * A raise must wait for it: a dismissal that is still queued (the app was + * killed before the ack reached the service) is the only record that this + * device already dismissed, and raising before the seed lands rings the alarm + * the user just dismissed — measured as "reopening the app retriggers it". + */ + const [dismissalsReady, setDismissalsReady] = useState(false); const markDismissed = useCallback((alertId: string, eventId?: string | null) => { dismissals.current.mark(alertId, eventId); @@ -148,7 +163,8 @@ export function AlarmProvider({ children }: { children: React.ReactNode }) { useEffect(() => { loadAcks() .then(list => dismissals.current.seed(list)) - .catch(() => {}); + .catch(() => {}) + .finally(() => setDismissalsReady(true)); }, []); // A per-install device id: stable across launches, unique per device. It @@ -242,10 +258,23 @@ export function AlarmProvider({ children }: { children: React.ReactNode }) { ); const endingFor = useCallback((alertId: string) => endings[alertId] ?? null, [endings]); + // Read by `handleTrigger`, which must not depend on the render that scheduled + // it: an alert this device saw end can never be raised again. + const endingsRef = useRef(endings); + endingsRef.current = endings; + const dismissalsReadyRef = useRef(dismissalsReady); + dismissalsReadyRef.current = dismissalsReady; const clearLastEnding = useCallback(() => setLastEnding(null), []); const handleTrigger = useCallback( (frame: { alertId: string; eventId?: string; message?: string; serverTime?: string }, live: boolean) => { + // Ended on this device (delivered, disarmed, expired). The alert is gone, + // so any frame that still names it is the tail of that alarm — the + // service's own status keeps answering for up to a poll, and a queued + // native launch can replay it. Raising it again is how a dismissed alarm + // came back with no title to show. + if (!dismissalsReadyRef.current) return; + if (endingsRef.current[frame.alertId]) return; // Dismissed on this device: the commanded status keeps saying "triggered" // until the owner disarms or everybody acks, so a status-driven raise for // the same event is not news. See lib/dismissals.ts. @@ -362,6 +391,10 @@ export function AlarmProvider({ children }: { children: React.ReactNode }) { const armedAttemptRef = useRef<{ alertId: string; at: number } | null>(null); useEffect(() => { if (!isAlarmNativeAvailable) return; + // Never start listening before the dismissals this device already made are + // known: the service's own check-in rings on whatever the ledger still owes, + // and an unsent dismissal would make it ring for an alarm just dismissed. + if (!dismissalsReady) return; if (!resolved || !online || !activeAlert?.subscribeToken) return; // The service holds one credential and cannot refresh an expiring access // token, so this path is the device-token one by construction. An @@ -404,6 +437,7 @@ export function AlarmProvider({ children }: { children: React.ReactNode }) { activeAlert, backgroundListening?.alertId, backgroundListening?.state, + dismissalsReady, online, refreshBackgroundListening, resolved, @@ -551,7 +585,6 @@ export function AlarmProvider({ children }: { children: React.ReactNode }) { */ const acknowledgeAlarm = useCallback( async (alertId: string, eventId: string | undefined) => { - if (!deviceId) return; // A dismissal whose event is unknown (the native takeover launch carries // only the alert) still names the trigger the status loop last reported; // otherwise the service clears this subscription's whole outstanding set. @@ -560,29 +593,52 @@ export function AlarmProvider({ children }: { children: React.ReactNode }) { // A subscriber acks with the device token it joined with; an account acks // with its session token (the api client picks the right header). const deviceToken = subscribedAlerts.find(a => a.alertId === alertId)?.deviceToken; - await enqueueAck({ alertId, eventId: known, deviceId, deviceToken }); - await flushAcks(); + // The device id is optional: a device-token ack identifies the + // subscription by its credential alone (measured against the service), and + // this used to `return` when the identity had not loaded yet — dropping the + // ack that keeps the alarm from ringing again after a restart. + await enqueueAck({ alertId, eventId: known, deviceId: deviceId ?? undefined, deviceToken }); + await flushAcks(deviceId ?? undefined); + // Dismissing is leaving: a subscriber that acknowledged has no reason to + // stay on the alert, and every leftover subscription is a live address + // that can ring again — the reported "it fires again when I reopen the + // app unless I unsubscribe". The ack settles the debt first (so the + // ledger is clean even if the removal fails), then the row goes. + const subscription = subscribedAlerts.find(a => a.alertId === alertId); + if (subscription?.deviceToken) { + try { + await api.removeSubscription( + { alertId, subscriptionId: subscription.subscriptionId ?? '' }, + subscription.deviceToken, + ); + } catch { + // Best effort: the debt is settled either way, so a failure only + // leaves a stale row (visible in "Subscribed devices") that the user + // can still remove from the alert. Never resurrect the alarm over it. + } + markEnded(alertId, 'left').catch(() => {}); + } }, - [deviceId, lastStatus?.eventId, markDismissed, subscribedAlerts], + [deviceId, lastStatus?.eventId, markDismissed, markEnded, subscribedAlerts], ); // Retry the durable queue on every plausible reconnect point, plus a slow // interval so a queue that could not flush at dismiss cannot linger. useEffect(() => { - flushAcks().catch(() => {}); + flushAcks(deviceId ?? undefined).catch(() => {}); const sub = AppState.addEventListener('change', next => { - if (next === 'active') flushAcks().catch(() => {}); + if (next === 'active') flushAcks(deviceId ?? undefined).catch(() => {}); }); - const timer = setInterval(() => flushAcks().catch(() => {}), 30_000); + const timer = setInterval(() => flushAcks(deviceId ?? undefined).catch(() => {}), 30_000); return () => { sub.remove(); clearInterval(timer); }; - }, []); + }, [deviceId]); useEffect(() => { - if (socketState === 'open') flushAcks().catch(() => {}); - }, [socketState]); + if (socketState === 'open') flushAcks(deviceId ?? undefined).catch(() => {}); + }, [socketState, deviceId]); const value: AlarmContextValue = { adminAlerts,