fix(focus-mode): prevent break skip when resuming from pause (#5995)

Replace time-based flag with store-based _isResumingBreak flag to
reliably distinguish break resume from manual tracking start.
This eliminates race conditions that caused breaks to be skipped.
This commit is contained in:
Johannes Millan 2026-01-19 18:27:50 +01:00
parent 2b5bf17027
commit b7139036f7
9 changed files with 573 additions and 22 deletions

View file

@ -0,0 +1,194 @@
/**
* E2E test for Bug #5995 - Resuming paused break starts next session
*
* This test will FAIL if the bug exists and PASS if it's fixed.
*
* Bug: When you pause a break from the banner and then resume it,
* the break gets skipped and the next Pomodoro work session starts.
*
* Expected: The break should continue from where it was paused.
*/
import { test, expect } from '../../fixtures/test.fixture';
import { Page } from '@playwright/test';
test.describe('Bug #5995: Resume paused break (CRITICAL BUG TEST)', () => {
let consoleLogs: string[] = [];
test.beforeEach(async ({ page }) => {
// Capture console logs for debugging
consoleLogs = [];
page.on('console', (msg) => {
const text = msg.text();
if (
text.includes('DEBUG Bug #5995') ||
text.includes('[a]') ||
text.includes('FocusMode')
) {
consoleLogs.push(text);
console.log(`CONSOLE: ${text}`);
}
});
});
test.afterEach(() => {
if (consoleLogs.length > 0) {
console.log('\n========== ALL CONSOLE LOGS ==========');
consoleLogs.forEach((log) => console.log(log));
console.log('======================================\n');
}
});
test('CRITICAL: Resuming paused break should continue break, not start next session', async ({
page,
}) => {
// Step 1: Enable the critical setting
await enableSyncSetting(page);
// Step 2: Create and track a task
await page.goto('/#/active/today');
await page.waitForSelector('task-list', { state: 'visible', timeout: 15000 });
const taskInput = page.locator('add-task-bar.global input');
await taskInput.fill('Bug5995CriticalTest');
await taskInput.press('Enter');
await page.waitForTimeout(500);
// Start tracking the task
const task = page.locator('task').first();
await task.hover();
const playBtn = page.locator('.play-btn.tour-playBtn').first();
await playBtn.click();
await page.waitForTimeout(500);
// Step 3: Start Pomodoro session
const focusButton = page
.getByRole('button')
.filter({ hasText: 'center_focus_strong' });
await focusButton.click();
await page.waitForTimeout(500);
const pomodoroButton = page.locator(
'segmented-button-group button:has-text("Pomodoro")',
);
await pomodoroButton.click();
await page.waitForTimeout(300);
const playButton = page.locator('focus-mode-main button.play-button');
await playButton.click();
// Wait for session to start
await page.waitForTimeout(5000);
// Step 4: Complete session to trigger break
const completeButton = page.locator('focus-mode-main .complete-session-btn');
await expect(completeButton).toBeVisible({ timeout: 10000 });
await completeButton.click();
// Wait for break to start
await page.waitForTimeout(2000);
// Step 5: Close overlay to show banner
const closeButton = page.locator('focus-mode-overlay button.close-btn');
await closeButton.click();
await page.waitForTimeout(500);
// Verify banner is visible with break running
const banner = page.locator('banner');
await expect(banner).toBeVisible({ timeout: 3000 });
await expect(banner).toContainText('Break', { timeout: 2000 });
console.log('\n=== STEP: About to pause break ===');
// Step 6: Pause the break
const pauseLink = banner.getByText('Pause', { exact: true });
await expect(pauseLink).toBeVisible({ timeout: 2000 });
await pauseLink.click();
await page.waitForTimeout(1000);
console.log('\n=== STEP: Break paused, about to resume ===');
// Step 7: Resume the break - THIS IS WHERE THE BUG HAPPENS
const resumeLink = banner.getByText('Resume', { exact: true });
await expect(resumeLink).toBeVisible({ timeout: 2000 });
await resumeLink.click();
// Wait for state to settle
await page.waitForTimeout(2000);
console.log('\n=== STEP: Checking result ===');
// Step 8: CRITICAL ASSERTION
// The banner should still show "Break" text (break is continuing)
// If the bug exists, it will show work session text instead
const bannerText = await banner.textContent();
console.log(`\n>>> BANNER TEXT AFTER RESUME: "${bannerText}"`);
// Check what's in the banner
const hasBreakText = bannerText?.includes('Break');
const hasSessionText =
bannerText?.includes('Session') || bannerText?.includes('Pomodoro');
console.log(`>>> Has 'Break' text: ${hasBreakText}`);
console.log(`>>> Has 'Session' text: ${hasSessionText}`);
// THE TEST: Break text should be present, session text should not
expect(hasBreakText).toBe(true);
expect(hasSessionText).toBe(false);
// Additional verification: Open overlay and check we're on break screen
await banner.getByText('To Focus Overlay').click();
await page.waitForTimeout(500);
const breakScreen = page.locator('focus-mode-break');
const mainScreen = page.locator('focus-mode-main');
await expect(breakScreen).toBeVisible({ timeout: 2000 });
await expect(mainScreen).not.toBeVisible();
});
});
const enableSyncSetting = async (page: Page): Promise<void> => {
console.log('\n=== STEP: Enabling sync setting ===');
await page.goto('/#/config');
await page.waitForLoadState('networkidle');
// Navigate to Productivity tab
const tabs = page.locator('[role="tab"]');
const productivityTab = tabs.filter({ hasText: /Productivity/i });
if ((await productivityTab.count()) > 0) {
await productivityTab.click();
await page.waitForTimeout(500);
}
// Find Focus Mode section
const sections = page.locator('config-section');
const focusModeSection = sections.filter({ hasText: 'Focus Mode' }).first();
// Try to expand it
const header = focusModeSection.locator('.section-header, h2, h3').first();
if (await header.isVisible({ timeout: 2000 }).catch(() => false)) {
await header.click();
await page.waitForTimeout(500);
}
// Find and enable the sync toggle
const syncText = 'Sync focus sessions with time tracking';
const toggles = page.locator('mat-slide-toggle');
const syncToggle = toggles.filter({ hasText: syncText }).first();
if (await syncToggle.isVisible({ timeout: 3000 }).catch(() => false)) {
const classes = await syncToggle.getAttribute('class');
if (!classes?.includes('mat-checked')) {
console.log('>>> Enabling sync setting...');
await syncToggle.click();
await page.waitForTimeout(500);
} else {
console.log('>>> Sync setting already enabled');
}
} else {
console.warn('>>> WARNING: Could not find sync toggle!');
}
};

View file

@ -204,6 +204,7 @@ describe('FocusModeModel', () => {
currentCycle: 0,
lastCompletedDuration: 0,
pausedTaskId: null,
_isResumingBreak: false,
};
expect(state.timer).toEqual(timer);

View file

@ -47,6 +47,9 @@ export interface FocusModeState {
// Task tracking during breaks
pausedTaskId: string | null;
// Internal flag: tracks if break resume is in progress
_isResumingBreak: boolean;
}
// Mode strategy interface

View file

@ -31,6 +31,9 @@ export const pauseFocusSession = createAction(
props<{ pausedTaskId?: string | null }>(),
);
export const unPauseFocusSession = createAction('[FocusMode] Resume Session');
export const clearResumingBreakFlag = createAction(
'[FocusMode] Clear Resuming Break Flag',
);
export const completeFocusSession = createAction(
'[FocusMode] Complete Session',

View file

@ -0,0 +1,339 @@
/**
* Integration tests for GitHub issue #5995
* https://github.com/super-productivity/super-productivity/issues/5995
*
* Bug #3: Resuming a paused break from banner starts next Pomodoro session
*
* When you:
* 1. Pause a Pomodoro break from the banner
* 2. Click Resume in the banner
*
* Expected: Break should continue from where it was paused
* Bug: Break is skipped and next work session starts
*
* Fix:
* Use store-based _isResumingBreak flag set by unPauseFocusSession reducer.
* When tracking starts during a break, check if it was caused by resuming (don't skip)
* vs user manually starting tracking (skip the break - preserves bug #5875 fix).
*/
import { TestBed, fakeAsync, tick, flush } from '@angular/core/testing';
import { provideMockActions } from '@ngrx/effects/testing';
import { BehaviorSubject, ReplaySubject } from 'rxjs';
import { FocusModeEffects } from './focus-mode.effects';
import { provideMockStore, MockStore } from '@ngrx/store/testing';
import { FocusModeStrategyFactory } from '../focus-mode-strategies';
import { GlobalConfigService } from '../../config/global-config.service';
import { TaskService } from '../../tasks/task.service';
import { BannerService } from '../../../core/banner/banner.service';
import { MetricService } from '../../metric/metric.service';
import { FocusModeStorageService } from '../focus-mode-storage.service';
import * as actions from './focus-mode.actions';
import * as selectors from './focus-mode.selectors';
import { FocusModeMode, FocusScreen, TimerState } from '../focus-mode.model';
import { setCurrentTask } from '../../tasks/store/task.actions';
import {
selectFocusModeConfig,
selectIsFocusModeEnabled,
} from '../../config/store/global-config.reducer';
import { Action } from '@ngrx/store';
describe('FocusMode Bug #5995: Resume paused break', () => {
let actions$: ReplaySubject<Action>;
let effects: FocusModeEffects;
let store: MockStore;
let strategyFactoryMock: any;
let taskServiceMock: any;
let globalConfigServiceMock: any;
let metricServiceMock: any;
let currentTaskId$: BehaviorSubject<string | null>;
const createMockTimer = (overrides: Partial<TimerState> = {}): TimerState => ({
isRunning: false,
startedAt: null,
elapsed: 0,
duration: 0,
purpose: null,
...overrides,
});
beforeEach(() => {
actions$ = new ReplaySubject<Action>(1);
currentTaskId$ = new BehaviorSubject<string | null>(null);
strategyFactoryMock = {
getStrategy: jasmine.createSpy('getStrategy').and.returnValue({
initialSessionDuration: 25 * 60 * 1000,
shouldStartBreakAfterSession: true,
shouldAutoStartNextSession: true,
getBreakDuration: jasmine
.createSpy('getBreakDuration')
.and.returnValue({ duration: 5 * 60 * 1000, isLong: false }),
}),
};
taskServiceMock = {
currentTaskId$: currentTaskId$.asObservable(),
currentTaskId: jasmine.createSpy('currentTaskId').and.returnValue(null),
};
globalConfigServiceMock = {
sound: jasmine.createSpy('sound').and.returnValue({ volume: 75 }),
};
metricServiceMock = {
logFocusSession: jasmine.createSpy('logFocusSession'),
};
TestBed.configureTestingModule({
providers: [
FocusModeEffects,
provideMockActions(() => actions$),
provideMockStore({
initialState: {
focusMode: {
timer: createMockTimer(),
mode: FocusModeMode.Pomodoro,
currentCycle: 1,
currentScreen: FocusScreen.Main,
mainState: 'preparation',
pausedTaskId: 'test-task-id',
lastCompletedDuration: null,
isOverlayShown: false,
_isResumingBreak: false,
},
tasks: {
entities: {
['test-task-id']: {
id: 'test-task-id',
title: 'Test Task',
projectId: null,
subTaskIds: [],
timeSpentOnDay: {},
timeSpent: 0,
timeEstimate: 0,
isDone: false,
created: Date.now(),
reminderId: null,
plannedAt: null,
_showSubTasksMode: 0,
attachments: [],
tagIds: [],
issueId: null,
issueWasUpdated: false,
issueLastUpdated: null,
issuePoints: null,
issueType: null,
issueAttachmentNr: null,
parentId: null,
notes: '',
dueDay: null,
repeatCfgId: null,
},
},
ids: ['test-task-id'],
currentTaskId: null,
lastCurrentTaskId: null,
selectedTaskId: null,
taskDetailPanelTargetPanel: null,
stateBefore: null,
isDataLoaded: true,
},
},
}),
{ provide: FocusModeStrategyFactory, useValue: strategyFactoryMock },
{ provide: TaskService, useValue: taskServiceMock },
{ provide: GlobalConfigService, useValue: globalConfigServiceMock },
{ provide: BannerService, useValue: {} },
{ provide: MetricService, useValue: metricServiceMock },
{ provide: FocusModeStorageService, useValue: {} },
],
});
effects = TestBed.inject(FocusModeEffects);
store = TestBed.inject(MockStore);
// Set up default selectors
store.overrideSelector(selectFocusModeConfig, {
isSkipPreparation: false,
isSyncSessionWithTracking: true,
isPauseTrackingDuringBreak: false,
});
store.overrideSelector(selectIsFocusModeEnabled, true);
});
afterEach(() => {
actions$.complete();
});
describe('Bug #3: Resume paused break should continue break', () => {
it('should NOT skip break when unPauseFocusSession is followed by tracking start', fakeAsync(() => {
// Setup: Break is paused (not running, purpose = break)
const pausedBreakTimer = createMockTimer({
purpose: 'break',
isRunning: false,
elapsed: 2 * 60 * 1000, // 2 minutes elapsed
duration: 5 * 60 * 1000, // 5 minute break
});
store.overrideSelector(selectors.selectTimer, pausedBreakTimer);
store.overrideSelector(selectors.selectMode, FocusModeMode.Pomodoro);
store.overrideSelector(selectors.selectCurrentScreen, FocusScreen.Main);
store.overrideSelector(selectors.selectPausedTaskId, 'test-task-id');
store.overrideSelector(selectors.selectIsResumingBreak, false); // Initially false
const dispatchedActions: Action[] = [];
effects.syncTrackingStartToSession$.subscribe((action) => {
dispatchedActions.push(action);
});
// Scenario: User clicks Resume in banner
// 1. unPauseFocusSession is dispatched (reducer sets _isResumingBreak = true)
actions$.next(actions.unPauseFocusSession());
// Simulate reducer updating the flag
store.overrideSelector(selectors.selectIsResumingBreak, true);
store.refreshState();
tick(10);
// 2. syncSessionResumeToTracking$ resumes tracking
currentTaskId$.next('test-task-id');
tick(10);
// Verify: No actions dispatched (EMPTY was returned, break continues)
// The clearResumingBreakFlag action is dispatched via store.dispatch(), not returned
expect(dispatchedActions.length).toBe(0);
flush();
}));
it('should skip break when user manually starts tracking (preserves bug #5875 fix)', fakeAsync(() => {
// Setup: Break is running
const runningBreakTimer = createMockTimer({
purpose: 'break',
isRunning: true,
elapsed: 2 * 60 * 1000,
duration: 5 * 60 * 1000,
});
store.overrideSelector(selectors.selectTimer, runningBreakTimer);
store.overrideSelector(selectors.selectMode, FocusModeMode.Pomodoro);
store.overrideSelector(selectors.selectCurrentScreen, FocusScreen.Main);
store.overrideSelector(selectors.selectPausedTaskId, 'test-task-id');
store.overrideSelector(selectors.selectIsResumingBreak, false); // No break resume in progress
const dispatchedActions: Action[] = [];
effects.syncTrackingStartToSession$.subscribe((action) => {
dispatchedActions.push(action);
});
// Scenario: User manually clicks play button on a task during break
// (NOT after unPauseFocusSession)
currentTaskId$.next('test-task-id');
tick(10);
// Verify: skipBreak was dispatched
expect(dispatchedActions.length).toBe(1);
expect(dispatchedActions[0].type).toBe(actions.skipBreak.type);
expect((dispatchedActions[0] as any).pausedTaskId).toBe('test-task-id');
flush();
}));
});
describe('syncSessionResumeToTracking$', () => {
it('should resume tracking when break is paused and unPauseFocusSession is dispatched', fakeAsync(() => {
// Setup: Paused break with pausedTaskId, tracking stopped
const pausedBreakTimer = createMockTimer({
purpose: 'break',
isRunning: false,
elapsed: 2 * 60 * 1000,
duration: 5 * 60 * 1000,
});
store.overrideSelector(selectors.selectTimer, pausedBreakTimer);
store.overrideSelector(selectors.selectPausedTaskId, 'test-task-id');
currentTaskId$.next(null); // Tracking is stopped
const dispatchedActions: Action[] = [];
effects.syncSessionResumeToTracking$.subscribe((action) => {
dispatchedActions.push(action);
});
// Action: Resume the break
actions$.next(actions.unPauseFocusSession());
tick(10);
// Verify: setCurrentTask was dispatched to resume tracking
expect(dispatchedActions.length).toBe(1);
expect(dispatchedActions[0].type).toBe(setCurrentTask.type);
expect((dispatchedActions[0] as any).id).toBe('test-task-id');
flush();
}));
it('should NOT resume tracking if tracking is already active', fakeAsync(() => {
// Setup: Paused break but tracking is already running
const pausedBreakTimer = createMockTimer({
purpose: 'break',
isRunning: false,
elapsed: 2 * 60 * 1000,
duration: 5 * 60 * 1000,
});
store.overrideSelector(selectors.selectTimer, pausedBreakTimer);
store.overrideSelector(selectors.selectPausedTaskId, 'test-task-id');
currentTaskId$.next('test-task-id'); // Tracking already running
const dispatchedActions: Action[] = [];
effects.syncSessionResumeToTracking$.subscribe((action) => {
dispatchedActions.push(action);
});
// Action: Resume the break
actions$.next(actions.unPauseFocusSession());
tick(10);
// Verify: NO action dispatched (tracking already active)
expect(dispatchedActions.length).toBe(0);
flush();
}));
it('should NOT resume tracking if sync is disabled', fakeAsync(() => {
// Setup: Sync disabled
store.overrideSelector(selectFocusModeConfig, {
isSkipPreparation: false,
isSyncSessionWithTracking: false,
isPauseTrackingDuringBreak: false,
});
const pausedBreakTimer = createMockTimer({
purpose: 'break',
isRunning: false,
elapsed: 2 * 60 * 1000,
duration: 5 * 60 * 1000,
});
store.overrideSelector(selectors.selectTimer, pausedBreakTimer);
store.overrideSelector(selectors.selectPausedTaskId, 'test-task-id');
currentTaskId$.next(null);
const dispatchedActions: Action[] = [];
effects.syncSessionResumeToTracking$.subscribe((action) => {
dispatchedActions.push(action);
});
// Action: Resume the break
actions$.next(actions.unPauseFocusSession());
tick(10);
// Verify: NO action dispatched (sync disabled)
expect(dispatchedActions.length).toBe(0);
flush();
}));
});
});

View file

@ -83,10 +83,13 @@ export class FocusModeEffects {
combineLatest([
this.store.select(selectFocusModeConfig),
this.store.select(selectIsFocusModeEnabled),
// Track break resume intent via store flag to distinguish between
// user manually starting tracking vs focus mode resuming tracking
this.store.select(selectors.selectIsResumingBreak),
]).pipe(
// Outer guard: skip config changes during sync
skipWhileApplyingRemoteOps(),
switchMap(([cfg, isFocusModeEnabled]) =>
switchMap(([cfg, isFocusModeEnabled, isResumingBreak]) =>
isFocusModeEnabled && cfg?.isSyncSessionWithTracking
? this.taskService.currentTaskId$.pipe(
// currentTaskId$ is local UI state (not synced), so distinctUntilChanged is sufficient
@ -103,14 +106,17 @@ export class FocusModeEffects {
if (timer.purpose === 'work' && !timer.isRunning) {
return of(actions.unPauseFocusSession());
}
// If break is active (running or paused), handle based on state
// Bug #5995 Fix: Don't skip paused breaks - resume them instead
// If break is active, handle based on state and cause
// Bug #5995 Fix: Don't skip breaks that were just resumed
if (timer.purpose === 'break') {
// If break is paused with time remaining, resume it
if (!timer.isRunning && timer.elapsed < timer.duration) {
return of(actions.unPauseFocusSession());
// Check store flag to distinguish between break resume and manual tracking start
if (isResumingBreak) {
// Clear flag after processing to prevent false positives
this.store.dispatch(actions.clearResumingBreakFlag());
return EMPTY; // Don't skip - break is resuming
}
// Break is running or completed, skip it (fixes bug #5875)
// User manually started tracking during break
// Skip the break to sync with tracking (bug #5875 fix)
return of(actions.skipBreak({ pausedTaskId }));
}
// If no session active, start a new one (only from Main screen)
@ -982,18 +988,6 @@ export class FocusModeEffects {
// Otherwise show play/pause button
const shouldShowStartButton = isSessionCompleted || isBreakTimeUp;
console.warn('🟢 [BUG 5995] Banner _getBannerActions:', {
timerRunning: timer.isRunning,
timerPurpose: timer.purpose,
timerElapsed: timer.elapsed,
timerDuration: timer.duration,
isPaused,
isOnBreak,
isSessionCompleted,
isBreakTimeUp,
shouldShowStartButton,
});
const playPauseAction = shouldShowStartButton
? {
label: T.F.FOCUS_MODE.B.START,

View file

@ -33,6 +33,7 @@ export const initialState: FocusModeState = {
currentCycle: 1,
lastCompletedDuration: 0,
pausedTaskId: null,
_isResumingBreak: false,
};
const createWorkTimer = (duration: number): TimerState => ({
@ -144,9 +145,16 @@ export const focusModeReducer = createReducer(
isRunning: true,
startedAt: Date.now() - state.timer.elapsed,
},
// Set flag ONLY when resuming a break (not work sessions)
_isResumingBreak: state.timer.purpose === 'break',
};
}),
on(a.clearResumingBreakFlag, (state) => ({
...state,
_isResumingBreak: false,
})),
on(a.completeFocusSession, (state, { isManual }) => {
const duration = state.timer.elapsed;

View file

@ -28,6 +28,7 @@ describe('FocusModeSelectors', () => {
currentCycle: 1,
lastCompletedDuration: 0,
pausedTaskId: null,
_isResumingBreak: false,
...overrides,
});
@ -151,11 +152,12 @@ describe('FocusModeSelectors', () => {
expect(result).toBe(false);
});
it('should return false for break sessions', () => {
it('should return true for paused break sessions (Bug #5995 fix)', () => {
const timer = createMockTimer({ isRunning: false, purpose: 'break' });
const result = selectors.selectIsSessionPaused.projector(timer);
expect(result).toBe(false);
// Bug #5995 fix: Breaks can be paused too
expect(result).toBe(true);
});
});

View file

@ -43,7 +43,8 @@ export const selectIsSessionRunning = createSelector(
export const selectIsSessionPaused = createSelector(
selectTimer,
(timer) => !timer.isRunning && timer.purpose === 'work',
// Bug #5995 fix: Detect pause for both work sessions AND breaks
(timer) => !timer.isRunning && (timer.purpose === 'work' || timer.purpose === 'break'),
);
// Break selectors
@ -90,3 +91,9 @@ export const selectPausedTaskId = createSelector(
selectFocusModeState,
(state) => state.pausedTaskId,
);
// Internal flag for break resume tracking
export const selectIsResumingBreak = createSelector(
selectFocusModeState,
(state) => state._isResumingBreak,
);