fix(android): ignore stale native focus-timer completion (#8805) (#8814)

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.
This commit is contained in:
Johannes Millan 2026-07-07 16:26:58 +02:00 committed by GitHub
parent 994f0522ff
commit 00d2073c27
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 121 additions and 8 deletions

View file

@ -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', () => {

View file

@ -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;