From 00d2073c2777ab89499c7cb8df132f587523dd2c Mon Sep 17 00:00:00 2001 From: Johannes Millan Date: Tue, 7 Jul 2026 16:26:58 +0200 Subject: [PATCH] fix(android): ignore stale native focus-timer completion (#8805) (#8814) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The native foreground-service completion is bridged into the store via handleNativeTimerComplete$. Its guard only checked purpose/isRunning, so a stale/late/duplicate native work-completion could complete a *different*, freshly-started work session — e.g. the one the user just advanced into from a break with the blue "next session" arrow — and in Pomodoro completing a work session auto-spawns a break, so the arrow appeared to start another break. Require the session to have reached its scheduled end by the WALL CLOCK (now - startedAt >= duration) rather than trusting the stored (and, while backgrounded, frozen) timer.elapsed. This rejects a completion landing on a just-started session while preserving the genuine over-run-on-resume case (#7856), where now - startedAt is already >= duration. Applied symmetrically to the break branch. Adds 5 regression tests for the stale/duplicate completion guard. --- .../store/android-focus-mode.effects.spec.ts | 70 +++++++++++++++++++ .../store/android-focus-mode.effects.ts | 59 +++++++++++++--- 2 files changed, 121 insertions(+), 8 deletions(-) diff --git a/src/app/features/android/store/android-focus-mode.effects.spec.ts b/src/app/features/android/store/android-focus-mode.effects.spec.ts index d6283dbe17..53885c9993 100644 --- a/src/app/features/android/store/android-focus-mode.effects.spec.ts +++ b/src/app/features/android/store/android-focus-mode.effects.spec.ts @@ -144,6 +144,76 @@ describe('shouldHandleNativeTimerComplete (native completion guard, #7856)', () }); }); +// A stale/duplicate native completion must not complete a *different* session +// than the one it was fired for. Landing on the fresh work session the user just +// advanced into (break -> "next session" arrow) would, in Pomodoro, immediately +// auto-spawn a break — the reported #8805 symptom. +describe('shouldHandleNativeTimerComplete (stale/duplicate completion guard, #8805)', () => { + const START = 1_000_000; + const WORK_DURATION = 25 * MIN; + const BREAK_DURATION = 5 * MIN; + + it('ignores a work completion on a work session that only just started (wall clock < duration)', () => { + const freshWork = workTimer(0, { startedAt: START }); + expect(shouldHandleNativeTimerComplete(false, freshWork, START + 500)).toBe(false); + }); + + it('handles a work completion once the session has reached its duration by wall clock, even with a frozen/stale elapsed (#7856 over-run)', () => { + // Backgrounded: stored elapsed frozen at 10 min, but 25 min of real time has + // passed since startedAt — the completion is genuine and must be handled. + const overrun = workTimer(10 * MIN, { startedAt: START }); + expect(shouldHandleNativeTimerComplete(false, overrun, START + WORK_DURATION)).toBe( + true, + ); + }); + + it('handles a work completion delivered slightly early (within tolerance)', () => { + const nearlyDone = workTimer(0, { startedAt: START }); + expect( + shouldHandleNativeTimerComplete(false, nearlyDone, START + WORK_DURATION - 500), + ).toBe(true); + }); + + it('ignores a break completion on a break that only just started', () => { + const freshBreak = workTimer(0, { + startedAt: START, + purpose: 'break', + duration: BREAK_DURATION, + }); + expect(shouldHandleNativeTimerComplete(true, freshBreak, START + 500)).toBe(false); + }); + + it('handles a break completion once the break has run its scheduled length', () => { + // Break stopped in-app (isRunning false) with the arrow showing; the native + // completion still auto-advances because the break reached its duration. + const doneBreak = workTimer(0, { + startedAt: START, + isRunning: false, + purpose: 'break', + duration: BREAK_DURATION, + }); + expect(shouldHandleNativeTimerComplete(true, doneBreak, START + BREAK_DURATION)).toBe( + true, + ); + }); + + it('ignores a completion when the timer has no startedAt (defensive)', () => { + // A running timer always has a startedAt in practice; guard defensively so a + // null can never pass the wall-clock check via `null` arithmetic. + const noStart = workTimer(0, { startedAt: null }); + expect(shouldHandleNativeTimerComplete(false, noStart, START + WORK_DURATION)).toBe( + false, + ); + }); + + it('ignores a work completion for a Flowtime session (duration 0 never schedules a native completion)', () => { + const flowtime = workTimer(0, { startedAt: START, duration: 0 }); + expect(shouldHandleNativeTimerComplete(false, flowtime, START + WORK_DURATION)).toBe( + false, + ); + }); +}); + // --- #7855: focus-session recovery helpers (see #7866) --- describe('AndroidFocusModeEffects helpers (#7855)', () => { describe('parseNativeFocusModeData', () => { diff --git a/src/app/features/android/store/android-focus-mode.effects.ts b/src/app/features/android/store/android-focus-mode.effects.ts index 9ca23b92a9..7e36d23d2a 100644 --- a/src/app/features/android/store/android-focus-mode.effects.ts +++ b/src/app/features/android/store/android-focus-mode.effects.ts @@ -60,21 +60,64 @@ export const hasFocusNotificationStateChanged = ( return Math.abs(currTimer.elapsed - prevTimer.elapsed) >= 5000; }; +/** + * Wall-clock slack (ms) for the "session has reached its scheduled end" check + * below. The native completion arrives at (or, after bridge/broadcast latency, + * just past) `startedAt + duration`, so a small positive slack absorbs latency + * and clock jitter without ever admitting the #8805 stale-completion case — a + * freshly-started session sits minutes away from its end, not seconds. + */ +const NATIVE_COMPLETE_TOLERANCE_MS = 2000; + /** * Whether a native timer-complete event should drive a state change. The native * foreground service fires this when its countdown reaches 0; we act on it only - * while the matching session is still active in app state — a break event needs an - * active break, a work event needs a still-running work session. The work guard is - * what makes the native completion a no-op once a resume `tick()` has already - * completed the session on return from the background (#7856), so the two never - * double-complete. Pure + exported so the `IS_ANDROID_WEB_VIEW`-gated effect's guard - * is unit-testable. + * while the matching session is still active in app state (a break event needs an + * active break, a work event a still-running work session) AND that session has + * actually reached its scheduled end by the WALL CLOCK (`now - startedAt >= + * duration`). + * + * The purpose/isRunning guard makes a work completion a no-op once a resume + * `tick()` has already completed the session on return from the background + * (#7856), so the two never double-complete. + * + * The wall-clock guard additionally rejects a STALE or duplicate native + * completion that lands on a *different*, still-running session than the one it + * was fired for — e.g. the work session the user just started by advancing from a + * break with the "next session" arrow. Without it that fresh session is completed + * immediately, and in Pomodoro completing a work session auto-spawns a break, so + * the arrow appears to "start another break" instead of the session (#8805). We + * compare against the wall clock rather than the stored `timer.elapsed` because a + * backgrounded session's `elapsed` is frozen and stale, whereas `now - startedAt` + * stays accurate — the same basis the reducer's `tick` uses (`elapsed = + * Date.now() - startedAt`), so a genuine over-run completion on resume (#7856) + * still passes even though its last in-app tick is far behind. + * + * `now` is injectable so the guard stays deterministically unit-testable. */ export const shouldHandleNativeTimerComplete = ( isBreak: boolean, timer: TimerState, -): boolean => - isBreak ? timer.purpose === 'break' : timer.purpose === 'work' && timer.isRunning; + now: number = Date.now(), +): boolean => { + // Must match the kind of session the event was fired for. + if (isBreak ? timer.purpose !== 'break' : timer.purpose !== 'work') { + return false; + } + // A work completion is void once the in-app tick already stopped the session + // (#7856). Breaks keep their prior semantics (no isRunning requirement): a + // finished break stays on-screen, stopped, until the user leaves it. + if (!isBreak && !timer.isRunning) { + return false; + } + // A running timer always has a startedAt, and fixed-duration timers have + // duration > 0 (Flowtime work — duration 0 — never schedules a native + // completion, so it never reaches here); guard defensively regardless. + if (timer.startedAt == null || timer.duration <= 0) { + return false; + } + return now - timer.startedAt >= timer.duration - NATIVE_COMPLETE_TOLERANCE_MS; +}; export type NativeFocusModeData = { durationMs: number;