From ad14668ffcd55da6af0c5358185a165a0060cb09 Mon Sep 17 00:00:00 2001 From: Mitsuki Fukunaga Date: Sun, 18 Jan 2026 10:53:52 +1100 Subject: [PATCH 1/4] test: add recovery notification cooldown coverage Add three test cases for 5-minute notification cooldown behavior: - Suppression within cooldown period (3 min < 5 min) - Allowed after cooldown expiration (6 min > 5 min) - First-time notification (lastRecoveryNotifiedAt undefined) Closes test coverage gap identified in PR #145 review. Co-Authored-By: Claude Sonnet 4.5 --- src/background/serviceWorker.test.ts | 120 +++++++++++++++++++++++++++ 1 file changed, 120 insertions(+) diff --git a/src/background/serviceWorker.test.ts b/src/background/serviceWorker.test.ts index 617e9ce..c77123e 100644 --- a/src/background/serviceWorker.test.ts +++ b/src/background/serviceWorker.test.ts @@ -267,6 +267,126 @@ describe('serviceWorker onInstalled event', () => { // Should clear pending flag expect(sessionRemoveMock).toHaveBeenCalledWith('pendingRecoveryNotification'); }); + + it('suppresses notification when within 5-minute cooldown', async () => { + const now = Date.now(); + const recentNotification = now - (3 * 60 * 1000); // 3 minutes ago (within cooldown) + + const sessionGetMock = vi.fn().mockResolvedValue({ + pendingRecoveryNotification: 5, + lastRecoveryNotifiedAt: recentNotification, + }); + const sessionSetMock = vi.fn().mockResolvedValue(undefined); + const sessionRemoveMock = vi.fn().mockResolvedValue(undefined); + const notificationsCreateMock = vi.fn().mockResolvedValue(undefined); + + (globalThis.chrome.storage.session.get as ReturnType) = sessionGetMock; + (globalThis.chrome.storage.session.set as ReturnType) = sessionSetMock; + (globalThis.chrome.storage.session.remove as ReturnType) = sessionRemoveMock; + (globalThis.chrome.notifications.create as ReturnType) = notificationsCreateMock; + + await importServiceWorker(); + + expect(installedHandler).not.toBeNull(); + + // Trigger onInstalled event + await installedHandler!(); + + // Should check for pending recovery notification + expect(sessionGetMock).toHaveBeenCalledWith(['pendingRecoveryNotification', 'lastRecoveryNotifiedAt']); + + // Should NOT create notification (within cooldown) + expect(notificationsCreateMock).not.toHaveBeenCalled(); + + // Should NOT update timestamp (notification suppressed) + expect(sessionSetMock).not.toHaveBeenCalled(); + + // Should still clear pending flag + expect(sessionRemoveMock).toHaveBeenCalledWith('pendingRecoveryNotification'); + }); + + it('shows notification when cooldown has expired', async () => { + const now = Date.now(); + const oldNotification = now - (6 * 60 * 1000); // 6 minutes ago (cooldown expired) + + const sessionGetMock = vi.fn().mockResolvedValue({ + pendingRecoveryNotification: 3, + lastRecoveryNotifiedAt: oldNotification, + }); + const sessionSetMock = vi.fn().mockResolvedValue(undefined); + const sessionRemoveMock = vi.fn().mockResolvedValue(undefined); + const notificationsCreateMock = vi.fn().mockResolvedValue(undefined); + + (globalThis.chrome.storage.session.get as ReturnType) = sessionGetMock; + (globalThis.chrome.storage.session.set as ReturnType) = sessionSetMock; + (globalThis.chrome.storage.session.remove as ReturnType) = sessionRemoveMock; + (globalThis.chrome.notifications.create as ReturnType) = notificationsCreateMock; + + await importServiceWorker(); + + expect(installedHandler).not.toBeNull(); + + // Trigger onInstalled event + await installedHandler!(); + + // Should check for pending recovery notification + expect(sessionGetMock).toHaveBeenCalledWith(['pendingRecoveryNotification', 'lastRecoveryNotifiedAt']); + + // Should create notification (cooldown expired) + expect(notificationsCreateMock).toHaveBeenCalledWith('recovery-notification', { + type: 'basic', + iconUrl: 'assets/icon128.png', + title: 'Snooooze Data Recovered', + message: 'Recovered 3 snoozed tabs from backup.', + priority: 1 + }); + + // Should update timestamp + expect(sessionSetMock).toHaveBeenCalledWith({ lastRecoveryNotifiedAt: expect.any(Number) }); + + // Should clear pending flag + expect(sessionRemoveMock).toHaveBeenCalledWith('pendingRecoveryNotification'); + }); + + it('shows notification when lastRecoveryNotifiedAt is undefined (first time)', async () => { + const sessionGetMock = vi.fn().mockResolvedValue({ + pendingRecoveryNotification: 2, + // lastRecoveryNotifiedAt is undefined (first time) + }); + const sessionSetMock = vi.fn().mockResolvedValue(undefined); + const sessionRemoveMock = vi.fn().mockResolvedValue(undefined); + const notificationsCreateMock = vi.fn().mockResolvedValue(undefined); + + (globalThis.chrome.storage.session.get as ReturnType) = sessionGetMock; + (globalThis.chrome.storage.session.set as ReturnType) = sessionSetMock; + (globalThis.chrome.storage.session.remove as ReturnType) = sessionRemoveMock; + (globalThis.chrome.notifications.create as ReturnType) = notificationsCreateMock; + + await importServiceWorker(); + + expect(installedHandler).not.toBeNull(); + + // Trigger onInstalled event + await installedHandler!(); + + // Should check for pending recovery notification + expect(sessionGetMock).toHaveBeenCalledWith(['pendingRecoveryNotification', 'lastRecoveryNotifiedAt']); + + // Should create notification (first time, no previous notification) + expect(notificationsCreateMock).toHaveBeenCalledWith('recovery-notification', { + type: 'basic', + iconUrl: 'assets/icon128.png', + title: 'Snooooze Data Recovered', + message: 'Recovered 2 snoozed tabs from backup.', + priority: 1 + }); + + // Should update timestamp + expect(sessionSetMock).toHaveBeenCalledWith({ lastRecoveryNotifiedAt: expect.any(Number) }); + + // Should clear pending flag + expect(sessionRemoveMock).toHaveBeenCalledWith('pendingRecoveryNotification'); + }); }); describe('serviceWorker onStartup event', () => { From 61f1734521018de848210a877cfa6c1db0286be2 Mon Sep 17 00:00:00 2001 From: Mitsuki Fukunaga Date: Sun, 18 Jan 2026 11:11:50 +1100 Subject: [PATCH 2/4] refactor: extract helper and add edge case tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Extract setupRecoveryNotificationTest helper to reduce duplication (120→100 lines) - Add boundary value test (exactly 5 minutes) - Add corruption case test (tabCount === 0) - Add singular message test (tabCount === 1) Addresses code quality and test coverage feedback from PR review. Co-Authored-By: Claude Sonnet 4.5 --- src/background/serviceWorker.test.ts | 164 +++++++++++++++++---------- 1 file changed, 101 insertions(+), 63 deletions(-) diff --git a/src/background/serviceWorker.test.ts b/src/background/serviceWorker.test.ts index c77123e..d0fe927 100644 --- a/src/background/serviceWorker.test.ts +++ b/src/background/serviceWorker.test.ts @@ -229,10 +229,12 @@ describe('serviceWorker onInstalled event', () => { expect(popCheck).toHaveBeenCalledOnce(); }); - it('checks for pending recovery notification', async () => { - const sessionGetMock = vi.fn().mockResolvedValue({ - pendingRecoveryNotification: 5, - }); + // Helper function to reduce test duplication + async function setupRecoveryNotificationTest(sessionData: { + pendingRecoveryNotification: number; + lastRecoveryNotifiedAt?: number; + }) { + const sessionGetMock = vi.fn().mockResolvedValue(sessionData); const sessionSetMock = vi.fn().mockResolvedValue(undefined); const sessionRemoveMock = vi.fn().mockResolvedValue(undefined); const notificationsCreateMock = vi.fn().mockResolvedValue(undefined); @@ -243,12 +245,16 @@ describe('serviceWorker onInstalled event', () => { (globalThis.chrome.notifications.create as ReturnType) = notificationsCreateMock; await importServiceWorker(); - expect(installedHandler).not.toBeNull(); - - // Trigger onInstalled event await installedHandler!(); + return { sessionGetMock, sessionSetMock, sessionRemoveMock, notificationsCreateMock }; + } + + it('checks for pending recovery notification', async () => { + const { sessionGetMock, sessionSetMock, sessionRemoveMock, notificationsCreateMock } = + await setupRecoveryNotificationTest({ pendingRecoveryNotification: 5 }); + // Should check for pending recovery notification expect(sessionGetMock).toHaveBeenCalledWith(['pendingRecoveryNotification', 'lastRecoveryNotifiedAt']); @@ -270,27 +276,13 @@ describe('serviceWorker onInstalled event', () => { it('suppresses notification when within 5-minute cooldown', async () => { const now = Date.now(); - const recentNotification = now - (3 * 60 * 1000); // 3 minutes ago (within cooldown) + const recentNotification = now - (3 * 60 * 1000); // 3 minutes ago (within 5-min cooldown) - const sessionGetMock = vi.fn().mockResolvedValue({ - pendingRecoveryNotification: 5, - lastRecoveryNotifiedAt: recentNotification, - }); - const sessionSetMock = vi.fn().mockResolvedValue(undefined); - const sessionRemoveMock = vi.fn().mockResolvedValue(undefined); - const notificationsCreateMock = vi.fn().mockResolvedValue(undefined); - - (globalThis.chrome.storage.session.get as ReturnType) = sessionGetMock; - (globalThis.chrome.storage.session.set as ReturnType) = sessionSetMock; - (globalThis.chrome.storage.session.remove as ReturnType) = sessionRemoveMock; - (globalThis.chrome.notifications.create as ReturnType) = notificationsCreateMock; - - await importServiceWorker(); - - expect(installedHandler).not.toBeNull(); - - // Trigger onInstalled event - await installedHandler!(); + const { sessionGetMock, sessionSetMock, sessionRemoveMock, notificationsCreateMock } = + await setupRecoveryNotificationTest({ + pendingRecoveryNotification: 5, + lastRecoveryNotifiedAt: recentNotification, + }); // Should check for pending recovery notification expect(sessionGetMock).toHaveBeenCalledWith(['pendingRecoveryNotification', 'lastRecoveryNotifiedAt']); @@ -307,27 +299,13 @@ describe('serviceWorker onInstalled event', () => { it('shows notification when cooldown has expired', async () => { const now = Date.now(); - const oldNotification = now - (6 * 60 * 1000); // 6 minutes ago (cooldown expired) + const oldNotification = now - (6 * 60 * 1000); // 6 minutes ago (exceeds 5-min cooldown) - const sessionGetMock = vi.fn().mockResolvedValue({ - pendingRecoveryNotification: 3, - lastRecoveryNotifiedAt: oldNotification, - }); - const sessionSetMock = vi.fn().mockResolvedValue(undefined); - const sessionRemoveMock = vi.fn().mockResolvedValue(undefined); - const notificationsCreateMock = vi.fn().mockResolvedValue(undefined); - - (globalThis.chrome.storage.session.get as ReturnType) = sessionGetMock; - (globalThis.chrome.storage.session.set as ReturnType) = sessionSetMock; - (globalThis.chrome.storage.session.remove as ReturnType) = sessionRemoveMock; - (globalThis.chrome.notifications.create as ReturnType) = notificationsCreateMock; - - await importServiceWorker(); - - expect(installedHandler).not.toBeNull(); - - // Trigger onInstalled event - await installedHandler!(); + const { sessionGetMock, sessionSetMock, sessionRemoveMock, notificationsCreateMock } = + await setupRecoveryNotificationTest({ + pendingRecoveryNotification: 3, + lastRecoveryNotifiedAt: oldNotification, + }); // Should check for pending recovery notification expect(sessionGetMock).toHaveBeenCalledWith(['pendingRecoveryNotification', 'lastRecoveryNotifiedAt']); @@ -349,35 +327,95 @@ describe('serviceWorker onInstalled event', () => { }); it('shows notification when lastRecoveryNotifiedAt is undefined (first time)', async () => { - const sessionGetMock = vi.fn().mockResolvedValue({ - pendingRecoveryNotification: 2, - // lastRecoveryNotifiedAt is undefined (first time) + const { sessionGetMock, sessionSetMock, sessionRemoveMock, notificationsCreateMock } = + await setupRecoveryNotificationTest({ + pendingRecoveryNotification: 2, + // lastRecoveryNotifiedAt is undefined (first time) + }); + + // Should check for pending recovery notification + expect(sessionGetMock).toHaveBeenCalledWith(['pendingRecoveryNotification', 'lastRecoveryNotifiedAt']); + + // Should create notification (first time, no previous notification) + expect(notificationsCreateMock).toHaveBeenCalledWith('recovery-notification', { + type: 'basic', + iconUrl: 'assets/icon128.png', + title: 'Snooooze Data Recovered', + message: 'Recovered 2 snoozed tabs from backup.', + priority: 1 }); - const sessionSetMock = vi.fn().mockResolvedValue(undefined); - const sessionRemoveMock = vi.fn().mockResolvedValue(undefined); - const notificationsCreateMock = vi.fn().mockResolvedValue(undefined); - (globalThis.chrome.storage.session.get as ReturnType) = sessionGetMock; - (globalThis.chrome.storage.session.set as ReturnType) = sessionSetMock; - (globalThis.chrome.storage.session.remove as ReturnType) = sessionRemoveMock; - (globalThis.chrome.notifications.create as ReturnType) = notificationsCreateMock; + // Should update timestamp + expect(sessionSetMock).toHaveBeenCalledWith({ lastRecoveryNotifiedAt: expect.any(Number) }); - await importServiceWorker(); + // Should clear pending flag + expect(sessionRemoveMock).toHaveBeenCalledWith('pendingRecoveryNotification'); + }); - expect(installedHandler).not.toBeNull(); + it('suppresses notification at exactly 5-minute boundary', async () => { + const now = Date.now(); + const NOTIFICATION_COOLDOWN = 5 * 60 * 1000; + const exactBoundary = now - NOTIFICATION_COOLDOWN; // Exactly 5 minutes - // Trigger onInstalled event - await installedHandler!(); + const { sessionGetMock, sessionSetMock, sessionRemoveMock, notificationsCreateMock } = + await setupRecoveryNotificationTest({ + pendingRecoveryNotification: 4, + lastRecoveryNotifiedAt: exactBoundary, + }); // Should check for pending recovery notification expect(sessionGetMock).toHaveBeenCalledWith(['pendingRecoveryNotification', 'lastRecoveryNotifiedAt']); - // Should create notification (first time, no previous notification) + // Should NOT create notification (boundary case: condition uses > not >=) + expect(notificationsCreateMock).not.toHaveBeenCalled(); + + // Should NOT update timestamp + expect(sessionSetMock).not.toHaveBeenCalled(); + + // Should still clear pending flag + expect(sessionRemoveMock).toHaveBeenCalledWith('pendingRecoveryNotification'); + }); + + it('shows corruption message when zero tabs recovered', async () => { + const { sessionGetMock, sessionSetMock, sessionRemoveMock, notificationsCreateMock } = + await setupRecoveryNotificationTest({ + pendingRecoveryNotification: 0, // Corruption case + }); + + // Should check for pending recovery notification + expect(sessionGetMock).toHaveBeenCalledWith(['pendingRecoveryNotification', 'lastRecoveryNotifiedAt']); + + // Should create notification with corruption message expect(notificationsCreateMock).toHaveBeenCalledWith('recovery-notification', { type: 'basic', iconUrl: 'assets/icon128.png', title: 'Snooooze Data Recovered', - message: 'Recovered 2 snoozed tabs from backup.', + message: 'Snoozed tabs data was reset due to corruption.', + priority: 1 + }); + + // Should update timestamp + expect(sessionSetMock).toHaveBeenCalledWith({ lastRecoveryNotifiedAt: expect.any(Number) }); + + // Should clear pending flag + expect(sessionRemoveMock).toHaveBeenCalledWith('pendingRecoveryNotification'); + }); + + it('uses singular "tab" when recovering one tab', async () => { + const { sessionGetMock, sessionSetMock, sessionRemoveMock, notificationsCreateMock } = + await setupRecoveryNotificationTest({ + pendingRecoveryNotification: 1, // Singular case + }); + + // Should check for pending recovery notification + expect(sessionGetMock).toHaveBeenCalledWith(['pendingRecoveryNotification', 'lastRecoveryNotifiedAt']); + + // Should create notification with singular "tab" (no 's') + expect(notificationsCreateMock).toHaveBeenCalledWith('recovery-notification', { + type: 'basic', + iconUrl: 'assets/icon128.png', + title: 'Snooooze Data Recovered', + message: 'Recovered 1 snoozed tab from backup.', priority: 1 }); From 517b99585a4bba4aaab2dd1b295060e086efe07e Mon Sep 17 00:00:00 2001 From: Mitsuki Fukunaga Date: Sun, 18 Jan 2026 11:16:32 +1100 Subject: [PATCH 3/4] docs: update implementation and user requirements descriptions in architecture and specifications --- dev-docs/ARCHITECTURE.md | 2 +- dev-docs/SPEC.md | 7 ++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/dev-docs/ARCHITECTURE.md b/dev-docs/ARCHITECTURE.md index 700b83b..ba03398 100644 --- a/dev-docs/ARCHITECTURE.md +++ b/dev-docs/ARCHITECTURE.md @@ -1,6 +1,6 @@ # Architecture -> Implementation details (structure, algorithms, file organization) +> Implementation details (structure, algorithms, data flow, invariants) Chrome Extension (Manifest V3) - Snooze tabs and restore at scheduled time diff --git a/dev-docs/SPEC.md b/dev-docs/SPEC.md index bb8fb90..37f1109 100644 --- a/dev-docs/SPEC.md +++ b/dev-docs/SPEC.md @@ -1,6 +1,6 @@ # Functional Specifications -> User requirements (behavior, timing, constraints) +> Feature requirements for product/QA (user behavior, timing, constraints) ## Terminology @@ -40,10 +40,12 @@ All times use user's timezone (settings → system fallback). ## Scope & Shortcuts **Scope:** + - **Selected Tabs**: Highlighted tabs, no `groupId` → restore in last-focused window - **Current Window**: All tabs, shared `groupId` → restore in new window **Keyboard:** + - Single key (e.g., `T`) → Snooze selected tabs - `Shift` + key → Snooze entire window - `Shift + P` or window scope → DatePicker preserves scope @@ -61,6 +63,7 @@ All times use user's timezone (settings → system fallback). ## UI Themes Defined in `constants.ts`: + - **Default**: Blue/Indigo monochrome - **Vivid**: Semantic colors (Tomorrow=Blue, Weekend=Green) - **Heatmap**: Urgency colors (Later Today=Red, Tomorrow=Orange) @@ -68,11 +71,13 @@ Defined in `constants.ts`: ## Data Integrity **Backup:** + - 3 rotating backups: `snoozedTabs_backup_` - Debounced 2s - Validates before backup **Recovery:** + - On startup: Validate `snoooze_v2` - If invalid → `recoverFromBackup` (valid → sanitized with most items → empty reset) - `ensureValidStorage` sanitizes invalid entries From d77cfa2fcf41be9a7037e6fd975b34e78596051b Mon Sep 17 00:00:00 2001 From: Mitsuki Fukunaga Date: Sun, 18 Jan 2026 12:10:20 +1100 Subject: [PATCH 4/4] docs: update CLAUDE.md to include code style guidelines --- .github/workflows/claude-code-review.yml | 14 ++------------ CLAUDE.md | 6 ++++++ 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/.github/workflows/claude-code-review.yml b/.github/workflows/claude-code-review.yml index 69a1eb1..5f2b75f 100644 --- a/.github/workflows/claude-code-review.yml +++ b/.github/workflows/claude-code-review.yml @@ -1,16 +1,7 @@ name: Claude Code Review -on: - pull_request: - types: [opened, ready_for_review, reopened] - # Optional: Only run on specific file changes - # paths: - # - "src/**/*.ts" - # - "src/**/*.tsx" - # - "src/**/*.js" - # - "src/**/*.jsx" - issue_comment: - types: [created] +# Temporarily disabled: no triggers. +on: [] jobs: claude-review: @@ -44,4 +35,3 @@ jobs: --allowed-tools "Read,Grep,Glob,Bash,Task,mcp__github_inline_comment__create_inline_comment" # See https://github.com/anthropics/claude-code-action/blob/main/docs/usage.md # for available options - diff --git a/CLAUDE.md b/CLAUDE.md index c4b66cc..0ce1f13 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -28,6 +28,12 @@ Chrome Extension (Manifest V3) - Snooze tabs and automatically restore them at a | `npm test` | Run all tests | | `npm run typecheck` | Type check | +## Code Style + +- TypeScript strict mode, no `any` types +- Use named exports, not default exports +- CSS: use Tailwind utility classes, no custom CSS files + ## Testing - External APIs must be mocked