[Feature][Medium] Reduce service worker technical debt: extract duplicated capture logic, remove dead code, externalize inline scripts

[numbers-official]

Overview

Deep check (2026-06-07) identified several significant technical debt items in the service worker and supporting services that impact maintainability, testability, and memory efficiency. These findings are distinct from existing issues #68 (memory/race conditions) and #69 (type divergence/monolithic component).

---

Finding 1: ~200 Lines of Duplicated Capture Logic

Files: src/background/service-worker.ts, lines 526–713 and 737–922
Priority: High

handleSelectionComplete() and handleScreenshotCapture() contain large blocks of nearly identical code: capturing screenshots via chrome.tabs.captureVisibleTab, watermarking via offscreen document, geolocation capture, website metadata extraction, asset creation/storage, auto-upload logic, and notification display.

Fix: Extract the shared capture pipeline into a single processCapture(source: 'selection' | 'screenshot', params) function. Each handler should only contain logic specific to its capture mode (e.g., cropping for selection, tab targeting for screenshot).

---

Finding 2: ScreenshotService.ts Is Unused Dead Code

File: src/services/ScreenshotService.ts (279 lines)
Priority: Medium

ScreenshotService is exported and has a singleton instance, but it is never imported by any other file in the codebase. The service worker handles screenshot capture directly via chrome.tabs.captureVisibleTab and the offscreen document. This entire file is dead code that may confuse contributors.

Fix: Remove the file, or integrate it as the actual capture abstraction (replacing the inline logic in the service worker).

---

Finding 3: 190-Line Content Script Defined Inline as String

File: src/background/service-worker.ts, lines 246–433
Priority: Medium

startProofSnapSelectionOverlay() defines a 190-line function as a string-injected content script inside the service worker file. It includes its own local types, event handlers, and DOM manipulation. This approach hinders linting, type checking, testing, and CSP compliance.

Fix: Move to a separate content script file registered in the manifest or injected via chrome.scripting.executeScript({ files: [...] }).

---

Finding 4: Default Settings Duplicated in Two Locations

Files: src/background/service-worker.ts, lines 51–68; src/services/StorageService.ts
Priority: Medium

Default settings are defined in both the onInstalled handler and StorageService.DEFAULT_SETTINGS. Adding a new setting requires updating both locations, which is error-prone and has likely led to drift.

Fix: Have the onInstalled handler call storageService.init() or reference StorageService.DEFAULT_SETTINGS as the single source of truth.

---

Finding 5: Math.random() Used for Asset ID Generation

File: src/background/service-worker.ts, line 630
Priority: Low

Asset ID generation uses Math.random().toString(36).slice(2, 11) for the random suffix. Math.random() is not cryptographically secure. While asset IDs are internal identifiers, the extension already uses crypto.getRandomValues() elsewhere (AuthService.ts line 91), so consistency is straightforward.

Fix: Replace with crypto.getRandomValues(new Uint8Array(8)) converted to a hex/base36 string.

---

Impact

• Eliminating the capture logic duplication reduces the service worker by ~200 lines and ensures bug fixes apply to both capture modes simultaneously.
• Removing dead code reduces cognitive overhead for contributors.
• Externalizing the inline content script enables linting, type checking, and CSP-compliant injection.
• Consolidating default settings prevents configuration drift.

Generated by Heart Beat with Omni