fix(extension): storage race fixes, mutex helper, and hardening sweep

[DYAI2025]

Summary

Builds on PR #10 (already merged) and PR #11 (Codex review). Fixes 7 code-review findings (C1–C3, H1, M1, M2, L1) plus 5 connected read-modify-write races discovered by a codebase-wide audit after the initial review.

Code-review findings fixed (from review of #10)

ID	Severity	Fix
C1	Critical	Serialize queue enqueue/drainHead/returnHead under a per-instance mutex
C2	Critical	Serialize dedup isFresh under a per-instance mutex
C3	Critical	retryNow returns all un-sent batches (failed + untried) to queue on auth_error
H1	High	Remove wildcard https://*/* host_permission and unused activeTab
M1	Medium	Byte-cap on queue (QUEUE_MAX_BYTES = 8 MB) prevents chrome.storage.session overflow
M2	Medium	Drop activeTab (already removed as part of H1)
L1	Low	Diagnostic export click handler wrapped in try/catch

Connected-race sweep (5 additional fixes)

After fixing C1/C2, an audit (grep -nE "await.*\.(get|query)\(") found the same anti-pattern in four more places:

Commit	Race site
e31c97d	Extract createMutex() to src/lib/mutex.js; rewire queue.js (5 sites) and dedup.js (2 sites)
4af5974	Router attempt counter (incrementAttempt / resetAttempt)
d4a8c10	saveConfig — module-level saveMutex
e3b1a43	Heartbeat counter — bumpHeartbeatCount + alarm read-runHeartbeat-write chain
3ee9331	acceptBatch now enqueues before clearRetry() on auth_error (consistency with retryNow)

New tests

• tests/lib/mutex.test.js — ordering, error-recovery, return-value forwarding
• tests/integration/heartbeat-race.test.js — 100 concurrent bumps with/without mutex (validates mock race fidelity)
• Additional router tests covering auth_error batch-preservation in both acceptBatch and retryNow

CI

106 tests, 99.17% statement coverage, 91.92% branch coverage ✅ (npm run ci — lint + typecheck + manifest + tests + coverage)

Test plan

• cd extension && npm run ci — all green
• Load unpacked in Chrome 122+, open WhatsApp Web, verify extension captures and forwards messages
• Set invalid API key, verify queued messages survive and replay after fix

Summary by Sourcery

Harden extension storage interactions against race conditions using a shared mutex helper, align router auth_error handling, and tighten manifest permissions while adding targeted tests and docs to cover the changes.

New Features:

• Introduce a reusable async mutex helper for serializing per-resource operations across the extension.
• Add a byte-based capacity limit to the send queue alongside the existing item-count cap.

Bug Fixes:

• Fix multiple read-modify-write races in the queue, deduplication, router attempt counter, config saving, and heartbeat counters by serializing access through mutexes.
• Ensure router retryNow and acceptBatch preserve appropriate batches in the queue on auth_error so they can be replayed after the user fixes credentials.
• Prevent diagnostic export failures in the popup from failing silently by surfacing errors to the user.

Enhancements:

• Refine router behavior so auth_error handling is consistent between normal sends and manual retries.
• Clarify storage behavior and limits in the extension README.
• Extract the lock pattern into a dedicated mutex utility to simplify and centralize concurrency control logic.

Documentation:

• Add detailed implementation plans documenting the connected race-condition sweep and PR bug-fix sweep.

Tests:

• Add unit tests for the mutex helper, queue byte-cap behavior, and concurrency behavior of queue and dedup under load.
• Extend router tests to cover auth_error batch preservation semantics and concurrent retry attempt counting.
• Add an integration test to validate heartbeat counter races and storage mock fidelity.
• Add manifest invariant tests to enforce minimal permissions and the absence of wildcard host permissions.

[sourcery-ai]

Reviewer's Guide

Introduces a reusable async mutex helper and applies it across storage-backed components (queue, dedup, router attempt counter, config, heartbeat) to eliminate read–modify–write races; hardens auth_error handling in the router so batches are preserved for replay; adds a byte cap to the send queue; tightens extension permissions and improves diagnostic UX; plus adds targeted tests and docs/plans to lock in the new concurrency and behavior contracts.

Sequence diagram for heartbeat bump and alarm handling with mutex

sequenceDiagram
  participant Background as BackgroundWorker
  participant HeartbeatMutex as HeartbeatMutex
  participant StorageLocal as StorageLocal
  participant Server as HeartbeatServer

  rect rgb(230,230,250)
    Background->>HeartbeatMutex: bumpHeartbeatCount(chatId)
    activate HeartbeatMutex
    HeartbeatMutex->>StorageLocal: get(HEARTBEAT_COUNTS_KEY)
    StorageLocal-->>HeartbeatMutex: counts
    HeartbeatMutex->>StorageLocal: set(updatedCounts)
    deactivate HeartbeatMutex
  end

  Note over Background,Server: Later, heartbeat alarm fires

  Background->>HeartbeatMutex: onHeartbeatAlarm()
  activate HeartbeatMutex
  HeartbeatMutex->>StorageLocal: get(HEARTBEAT_COUNTS_KEY)
  StorageLocal-->>HeartbeatMutex: counts
  HeartbeatMutex->>Server: runHeartbeat(serverUrl, apiKey, counts, queueSize)
  Server-->>HeartbeatMutex: result.remaining
  HeartbeatMutex->>StorageLocal: set({HEARTBEAT_COUNTS_KEY: result.remaining})
  deactivate HeartbeatMutex

  Note over HeartbeatMutex: Mutex ensures bumps and alarm updates do not race

Loading

Sequence diagram for router auth_error handling in acceptBatch and retryNow

sequenceDiagram
  actor Popup as PopupOrContent
  participant Router as Router
  participant Queue as SendQueue
  participant Transport as Transport
  participant Retry as RetryScheduler
  participant StorageSession as StorageSession

  Popup->>Router: acceptBatch(messages)
  Router->>Transport: sendBatch(cfg, messages)
  Transport-->>Router: outcome=auth_error
  Router->>Queue: enqueue(messages)
  Router->>Retry: clearRetry()
  Router-->>Popup: outcome=rejected, reason=auth_error

  Note over Router,Queue: Batch is preserved in queue for replay

  Popup->>Router: retryNow()
  Router->>Queue: drainHead(10)
  Queue-->>Router: headBatches
  loop for each batch in headBatches
    Router->>Transport: sendBatch(cfg, batch)
    Transport-->>Router: outcome
    alt outcome==auth_error
      break auth_error
        Router->>Queue: returnHead(failedBatchesAndUntriedTail)
        Router->>Retry: clearRetry()
        Router->>StorageSession: resetAttempt()
        Router-->>Popup: outcome=auth_error
      end
    else outcome==error
      Router->>Queue: trackFailed(batch)
    end
  end
  alt noAuthErrorAndSomeFailed
    Router->>Queue: returnHead(failedBatches)
    Router->>Retry: scheduleRetry(backoff)
    Router-->>Popup: outcome=partial
  else allOk
    Router->>StorageSession: resetAttempt()
    Router->>Retry: clearOrReschedule()
    Router-->>Popup: outcome=ok
  end

Loading

Class diagram for mutex-based serialization across storage components

classDiagram
  class Mutex {
    +run(fn)
  }

  class Queue {
    +enqueue(item)
    +drainHead(n)
    +returnHead(items)
    +clear()
    +size()
    +droppedCount()
    +resetDroppedCount()
    -store
    -mutex
    -maxSize
    -maxBytes
  }

  class Dedup {
    +isFresh(id)
    +size()
    +clear()
    -store
    -mutex
    -key
    -windowSize
  }

  class ConfigStore {
    +loadConfig()
    +saveConfig(patch)
    -saveMutex
  }

  class Router {
    +createRouter()
    +acceptBatch(messages)
    +retryNow()
    +snapshot()
    -queue
    -attemptMutex
  }

  class HeartbeatManager {
    +bumpHeartbeatCount(chatId)
    +onHeartbeatAlarm()
    -heartbeatMutex
  }

  Mutex <.. Queue : uses
  Mutex <.. Dedup : uses
  Mutex <.. ConfigStore : uses
  Mutex <.. Router : attemptMutex
  Mutex <.. HeartbeatManager : heartbeatMutex

  class StorageSession {
    +get(keys)
    +set(items)
  }

  class StorageLocal {
    +get(keys)
    +set(items)
  }

  Queue --> StorageSession : uses
  Router --> StorageSession : attemptCounter
  HeartbeatManager --> StorageLocal : heartbeatCounts
  Dedup --> StorageLocal : default
  ConfigStore --> StorageLocal : config

Loading

File-Level Changes

Change	Details	Files
Extracts a generic async mutex helper and reimplements queue and dedup locking on top of it, then strengthens their concurrency test coverage.	Adds src/lib/mutex.js implementing a per-resource promise-chain mutex with a run(fn) API that preserves ordering and handles errors without breaking the chain. Refactors queue.js to use createMutex for all mutating operations (enqueue, drainHead, returnHead, clear, resetDroppedCount) and extends enqueue with an optional maxBytes eviction path while keeping size/peek/droppedCount as simple reads. Refactors dedup.js to use createMutex around isFresh and clear operations so concurrent calls cannot double-accept the same id, while leaving size as a read-only operation. Extends queue and dedup unit tests with concurrency-focused cases (parallel enqueues/drains for queue, concurrent isFresh calls for dedup) and a new queue maxBytes test to validate byte capping and droppedCount behavior. Adds dedicated mutex unit tests (ordering, error recovery, return-value propagation) to validate the helper in isolation.	extension/src/lib/mutex.js extension/src/lib/queue.js extension/src/lib/dedup.js extension/tests/lib/mutex.test.js extension/tests/lib/queue.test.js extension/tests/lib/dedup.test.js
Hardens router behavior around auth_error and retry attempts, including attempt-counter serialization, queue byte-cap wiring, and consistent batch preservation semantics between acceptBatch and retryNow, with new tests.	Introduces a QUEUE_MAX_BYTES constant (8 MB) and passes it into createQueue so the router’s session-backed queue respects both count and byte caps. Adds a per-router attemptMutex and rewrites incrementAttempt and resetAttempt as closure helpers that run their read–modify–write sequences under the mutex while keeping getAttempt as a simple read. Changes retryNow’s drain loop to track index, and on auth_error returns both earlier failed batches and not-yet-tried batches to the queue via queue.returnHead, aligning with the documented contract that only the auth-rejected batch is dropped. Updates acceptBatch’s auth_error path to enqueue the batch before clearing the retry alarm so that batches rejected due to bad credentials are preserved for replay after the user fixes the key, keeping behavior consistent with retryNow. Expands router tests to cover auth_error queue preservation for both acceptBatch and retryNow (including mixed server_error/auth_error flows and concurrent retryNow calls stressing the attempt counter).	extension/src/lib/router.js extension/tests/lib/router.test.js
Serializes config saves and heartbeat count updates under dedicated mutexes to eliminate chrome.storage read–modify–write races, and adds integration tests to validate heartbeat race behavior.	Imports createMutex into config.js and wraps saveConfig in a module-level saveMutex so concurrent patches cannot clobber each other when writing the config record. Extends config tests with scenarios where parallel saveConfig calls touch overlapping and disjoint fields to ensure patches compose instead of being lost. Imports createMutex into background.js, adds a heartbeatMutex, and wraps both bumpHeartbeatCount and the heartbeat alarm handler’s runHeartbeat read–write chain under the same mutex to prevent lost increments during heartbeat sends. Adjusts bumpHeartbeatCount to return a promise (or a resolved promise when chatId is falsy) so callers can await the serialized update path. Adds an integration test file that simulates concurrent heartbeat counter bumps using both a mutex-protected and an unprotected implementation to demonstrate the race and validate the fix under the chrome.storage mock.	extension/src/lib/config.js extension/tests/lib/config.test.js extension/background.js extension/tests/integration/heartbeat-race.test.js
Tightens extension permissions and manifest invariants by dropping unused/overscoped permissions and asserting them in tests.	Removes the activeTab permission from manifest.json since content scripts are auto-injected via manifest matches and no executeScript-based flows exist. Removes the https:/// host_permissions wildcard, narrowing host access to the specific WhatsApp Web and localhost origins. Updates manifest tests to assert that permissions are limited to alarms and storage, that host_permissions does not contain protocol or catch-all wildcards, and that activeTab is not requested.	extension/manifest.json extension/tests/manifest.test.js
Improves diagnostic and documentation UX around queue behavior and error handling, including popup diagnostic export robustness and updated storage documentation.	Wraps the diagnostic export button handler in popup.js in a try/catch, surfacing failures via setStatus instead of silently dropping errors when collectDiagnostics or Blob/URL operations fail. Updates the README storage section to describe the combined QUEUE_MAX and QUEUE_MAX_BYTES constraints, clarifying how storage.session quota is respected and how droppedCount reflects byte-based evictions, particularly for large audio messages. Adds detailed implementation-plan documents for the connected race sweep and the PR feat(extension): durable queue, modular ESM, and CI hardening #10 bug-fix sweep, capturing rationale, task breakdowns, and verification steps for future reviewers and maintainers.	extension/popup.js extension/README.md docs/plans/2026-04-30-connected-races-sweep.md docs/plans/2026-04-30-pr10-bugfix-sweep.md

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[sourcery-ai]

Hey - I've found 1 issue, and left some high level feedback:

• In the mutex helper, run currently always calls fn as tail.then(fn, fn) but its type is () => Promise<T>; consider either updating the signature to accept an argument or using tail.then(() => fn(), () => fn()) to avoid passing through the previous result and keep the implementation aligned with the declared type.
• The queue enqueue byte-cap logic repeatedly calls JSON.stringify(arr) inside a loop, which is O(n²) in the worst case for large queues; it may be worth tracking an approximate byte size incrementally or computing the length once per enqueue to avoid redundant full re-serializations.
• The new parallel saveConfig test uses globalThis.__lastWhitelist without ever assigning to it, which makes the intent a bit confusing; either wire this up properly or simplify the test to more directly exercise the serialized patch behavior.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- In the mutex helper, `run` currently always calls `fn` as `tail.then(fn, fn)` but its type is `() => Promise<T>`; consider either updating the signature to accept an argument or using `tail.then(() => fn(), () => fn())` to avoid passing through the previous result and keep the implementation aligned with the declared type.
- The queue `enqueue` byte-cap logic repeatedly calls `JSON.stringify(arr)` inside a loop, which is O(n²) in the worst case for large queues; it may be worth tracking an approximate byte size incrementally or computing the length once per enqueue to avoid redundant full re-serializations.
- The new `parallel saveConfig` test uses `globalThis.__lastWhitelist` without ever assigning to it, which makes the intent a bit confusing; either wire this up properly or simplify the test to more directly exercise the serialized patch behavior.

## Individual Comments

### Comment 1
<location path="extension/tests/lib/config.test.js" line_range="62-71" />
<code_context>
+  it('parallel saveConfig calls do not lose patches', async () => {
</code_context>
<issue_to_address>
**issue (testing):** Race test for `saveConfig` is misleading and relies on an undefined `__lastWhitelist` helper

As written, this test doesn’t model a real read–modify–write flow:

- `globalThis.__lastWhitelist` is never assigned in the test, so every `saveConfig` call sees `undefined` and writes `[name]`; you never exercise “append to existing whitelist”, only concurrent overwrites.
- The final assertion `cfg.whitelist.length >= 1` is too weak and would still pass if all but one write were lost.

I’d either remove this test to avoid a false sense of safety, or rewrite it so each concurrent call explicitly `loadConfig()`, appends a unique marker, then calls `saveConfig`, and you finally assert that all markers are present (order-agnostic, no duplicates). In either case, dropping the unused `globalThis.__lastWhitelist` indirection would make the test clearer.
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[sourcery-ai]

issue (testing): Race test for saveConfig is misleading and relies on an undefined __lastWhitelist helper

As written, this test doesn’t model a real read–modify–write flow:

• globalThis.__lastWhitelist is never assigned in the test, so every saveConfig call sees undefined and writes [name]; you never exercise “append to existing whitelist”, only concurrent overwrites.
• The final assertion cfg.whitelist.length >= 1 is too weak and would still pass if all but one write were lost.

I’d either remove this test to avoid a false sense of safety, or rewrite it so each concurrent call explicitly loadConfig(), appends a unique marker, then calls saveConfig, and you finally assert that all markers are present (order-agnostic, no duplicates). In either case, dropping the unused globalThis.__lastWhitelist indirection would make the test clearer.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c7001afc3a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Re-add host access for configured HTTPS API endpoints

By removing https://*/*, the extension now only has host access to WhatsApp and localhost, but the UI and config model still allow arbitrary backend URLs (for example https://radar.example.com). In that common deployment case, background fetch calls in the ingest/heartbeat paths will lack required host permission and fail, so captures are never delivered and the retry queue will just grow. This should be replaced with an optional_host_permissions request flow or another mechanism that grants host access to the user-configured server.

Useful? React with 👍 / 👎.

[Copilot]

Pull request overview

This PR hardens the extension’s durability and correctness by eliminating several chrome.storage read-modify-write races via a shared mutex helper, tightening manifest permissions, and aligning auth_error handling so queued messages are preserved for replay.

Changes:

• Introduce createMutex() and apply it to queue/dedup/config/attempt counter/heartbeat counters to prevent lost updates under concurrency.
• Add an 8 MB serialized byte cap for the send queue (in addition to the existing batch-count cap) to reduce chrome.storage.session quota risk.
• Tighten manifest permissions/host permissions and expand tests (mutex unit tests, multiple concurrency regression tests, manifest invariants).

Reviewed changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
extension/tests/manifest.test.js	Updates permission expectations and adds host wildcard invariants.
extension/tests/lib/router.test.js	Adds/updates tests for auth_error preservation and attempt-counter concurrency.
extension/tests/lib/queue.test.js	Adds concurrency tests and a max-bytes eviction test for the queue.
extension/tests/lib/mutex.test.js	New unit tests for mutex ordering, error recovery, and return values.
extension/tests/lib/dedup.test.js	Adds concurrency test to ensure isFresh() returns true only once.
extension/tests/lib/config.test.js	Adds concurrency-focused tests for saveConfig serialization behavior.
extension/tests/integration/heartbeat-race.test.js	New regression tests demonstrating heartbeat counter races with/without mutex.
extension/src/lib/router.js	Adds queue byte cap wiring, serializes attempt counter updates, and changes auth_error handling.
extension/src/lib/queue.js	Adds per-instance mutex and optional maxBytes eviction.
extension/src/lib/mutex.js	New shared mutex helper used across modules to serialize async sections.
extension/src/lib/dedup.js	Wraps isFresh()/clear() in a mutex to prevent concurrent duplicates.
extension/src/lib/config.js	Serializes saveConfig() with a module-level mutex to prevent patch clobbering.
extension/popup.js	Wraps diagnostic export flow in try/catch and surfaces failures to the user.
extension/manifest.json	Removes activeTab and the https://*/* host wildcard.
extension/background.js	Serializes heartbeat counter updates and the alarm flush chain with a mutex.
extension/README.md	Updates storage quota guidance to reflect the new dual (count + bytes) queue caps.
docs/plans/2026-04-30-pr10-bugfix-sweep.md	Adds implementation plan documentation for the original PR10 sweep.
docs/plans/2026-04-30-connected-races-sweep.md	Adds implementation plan documentation for the connected races sweep.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.