Skip to content

fix(desktop): clear Cache Storage when deleting browser profile data - #22002

Open
only21mil wants to merge 2 commits into
pingdotgg:mainfrom
only21mil:r6/s06-2
Open

only21mil wants to merge 2 commits into
pingdotgg:mainfrom
only21mil:r6/s06-2

Conversation

@only21mil

@only21mil only21mil commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

Problem

In the desktop app, Settings → Integrations → Browser profiles → Clear cookies and cache reports "Cleared 's cookies and cache", and removing a profile runs the same clear. Neither removes the page's Cache Storage (caches / Cache API). After the clear, a site's cached responses are still there the next time the profile opens it.

BrowserSession.clearCookies passes cookies, localstorage, indexdb and serviceworkers to session.clearStorageData, then the clear action calls session.clearCache(). Electron's clearCache() only empties the HTTP cache. Cache Storage is its own clearStorageData type, cachestorage, and nothing passes it.

Steps: open a page in a profile's browser tab, run caches.open("demo-cache").then((c) => c.put("/x", new Response("x"))), clear the profile's data from Settings, reload the tab. caches.keys() still returns ["demo-cache"]. localStorage from the same page is gone.

Change

Adds "cachestorage" to the storages list in BrowserSession.clearCookies. Partition selection is unchanged, so a clear still touches only the partitions it touched before.

Cache Storage goes with the site data that call already clears (local storage, IndexedDB, service workers), not with the HTTP cache, the same grouping Chromium uses in Clear browsing data. The tab menu's Clear cookies item uses the same call, so it now also removes Cache Storage for that profile, as it already does for IndexedDB and service workers.

Scope and approval

No issue. I'm submitting this under the small obvious-fix exception in CONTRIBUTING.md: the action says it clears the profile's cookies and cache, and it already clears cookies, local storage, IndexedDB and service workers, but not Cache Storage. It's a one-line production change with no new setting or default.

Prior PRs: none fix this. #8256 was closed for missing prior approval because it moved preview storage to per-project partitions. This PR doesn't change partitions or which profiles a clear touches.

Verification

Real app, desktop dev build on Linux (Electron 44.4.5), Default profile, local page that reads Cache Storage on load. I seeded one Cache Storage entry and one localStorage key from the tab, ran Settings → Integrations → Default → Clear cookies and cache → Clear data, then reloaded the tab.

main: localStorage is cleared, Cache Storage still holds demo-cache with 1 entry.

App on main after Clear data

This branch: caches.keys() is [], 0 entries.

App on this branch after Clear data

Both runs used the same app session. Between them I swapped only BrowserSession.ts, and the rebuilt bundle showed the matching storages list each time.

Focused test: vp test run src/preview/BrowserSession.test.ts in apps/desktop, 8/8 pass. #22557 deleted the test that pinned the storages list, so this PR no longer edits a test. Restored outside the PR, that test fails with main's BrowserSession.ts and passes on this branch. The screenshot is from the earlier head, before #22557.

Test against main's list and this branch

Partition isolation, plain Electron 44.4.5 with two persist: partitions shaped like profile partitions: seed both, clear profile A with the same two calls the clear action makes, restart, read both. With main's list profile A keeps its Cache Storage. With this list it's empty. Profile B keeps its data in both runs.

Two partitions in plain Electron

Also ran tsc-rs --noEmit for apps/desktop, plus vp lint and vp fmt --check on the changed file. All passed.

Not checked:

  • macOS and Windows. The storage type is Electron's, so I don't expect a difference, but I only ran Linux.
  • Removing a profile end to end. It calls the same clearBrowserProfileData as the clear button, which I did run.
  • Server-hosted browser tabs. They use a separate clear path that this PR doesn't touch.
  • No repository test drives real Electron storage or pins the storages list. The app and Electron runs above show the effect.

Production fix, PR description and images written with Claude Opus 5.5 in T3 Code Nightly.

🤖 Generated with Claude Opus 5.5 in T3 Code Nightly

Clearing a profile's cookies already removed local storage, IndexedDB and
service workers, but left the page's Cache API store behind. Electron's
clearCache() only empties the HTTP cache, so "Clear data" and profile
removal kept Cache Storage entries in the profile's partition.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:XS 0-9 changed lines (additions + deletions). labels Oct 19, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 19, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 04ac632

Macroscope's review found this PR approvable — This narrowly scoped fix adds Cache Storage to the existing browser-profile data-clearing operation and updates its focused test. It preserves partition selection and all other clearing behavior, with no new settings, schema changes, or static-analysis suppressions.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Path: .coderabbit.config.ts
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 3c9de15b-5bb0-4421-8068-5c683fc41241
📥 Commits

Reviewing files that changed from the base of the PR and between 48b486b and 04ac632.

📒 Files selected for processing: 2
  • apps/desktop/src/preview/BrowserSession.test.ts
  • apps/desktop/src/preview/BrowserSession.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.

📝 Walkthrough

BrowserSession.clearCookies now clears the cachestorage storage type along with cookies, local storage, IndexedDB, and service workers. The test expectation includes cachestorage.

Changes: Preview storage clearing

Layer / File(s) Summary
Include cache storage
apps/desktop/src/preview/BrowserSession.ts, apps/desktop/src/preview/BrowserSession.test.ts
clearCookies adds cachestorage to the storage types it clears. The test expectation includes the added type.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~5 minutes

Change: Bug fix

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to 04ac6

The change clears Cache Storage for the selected browser profile alongside the existing site data. No merge-blocking risk remains.

Architecture risk: 🔵 Low · up to 04ac6

The change affects 1 system.

Changed systems: apps/desktop

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — apps/desktop (service) was modified; 2 changed files map to changed impact.

Before / after behavior

  • observed — Modified behavior in apps/desktop/src/preview/BrowserSession.test.ts: The expected storage list adds cachestorage; the prior assertion omitted it.
  • observed — Modified behavior in apps/desktop/src/preview/BrowserSession.ts: clearCookies now includes cachestorage in the storage types cleared; the previous list omitted it.
Pre-merge checks: Passed 4
Check name Status Explanation
Title check Passed The title clearly and concisely identifies the main change: clearing Cache Storage when deleting desktop browser profile data.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Description check Passed The pull request includes all required sections: Problem, Change, Scope and approval, and Verification. It explains the issue, implementation, approval exception, test results, manual validation, limi…
Full details: Description check

Explanation

The pull request includes all required sections: Problem, Change, Scope and approval, and Verification. It explains the issue, implementation, approval exception, test results, manual validation, limitations, and agent attribution.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XS 0-9 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant