fix(webview-accounts): retry data-dir purge so CEF handle race doesn't leak cookies (#1076)#1081
Conversation
…t leak cookies (tinyhumansai#1076) `webview_account_purge` calls `wv.close()` then immediately `remove_dir_all` on the per-account data directory. CEF / WKWebView holds file handles briefly after close returns, so the deletion races the subprocess and fails on macOS — leaving cookies on disk. Re-adding the same provider then lands the user already signed in (the symptom user reported across Slack tinyhumansai#1016, Gmail tinyhumansai#1020, Gmeet tinyhumansai#1022 audits). Extract the deletion into `purge_data_dir_with_retry`: 5 attempts with 100ms / 200ms / 400ms / 800ms / 1600ms exponential backoff, debug log per failed attempt, warn log only after all attempts exhausted. The total ~3.1s budget covers the typical macOS handle-release window (~200-500ms) with margin to spare. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughEnhanced the account data directory purge operation in Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Review rate limit: 0/5 reviews remaining, refill in 6 minutes and 47 seconds. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src-tauri/src/webview_accounts/mod.rs (1)
2083-2090:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPropagate terminal purge failure instead of always reporting success
Line 2085 logs a successful purge and the command returns
Ok(())even when Lines 2132-2137 logged that deletion failed after all retries. That makes explicit “Remove account” look successful while cookies may still persist.Suggested fix
-async fn purge_data_dir_with_retry(data_dir: &std::path::Path) { +async fn purge_data_dir_with_retry(data_dir: &std::path::Path) -> Result<(), std::io::Error> { if !data_dir.exists() { - return; + return Ok(()); } @@ - return; + return Ok(()); } @@ Err(err) => { log::warn!( "[webview-accounts] purge remove_dir_all {} failed after {} attempts: {} — cookies may persist; cross-launch fallback handled by schedule_cef_profile_purge", data_dir.display(), MAX_ATTEMPTS, err ); + return Err(err); } } } + Ok(()) }- purge_data_dir_with_retry(&data_dir).await; - - log::info!( - "[webview-accounts] purged account={} label={:?}", - args.account_id, - label_opt - ); - Ok(()) + purge_data_dir_with_retry(&data_dir) + .await + .map_err(|e| format!("purge data dir {}: {e}", data_dir.display()))?; + + log::info!( + "[webview-accounts] purged account={} label={:?}", + args.account_id, + label_opt + ); + Ok(()) }Also applies to: 2101-2141
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src-tauri/src/webview_accounts/mod.rs` around lines 2083 - 2090, The code always logs success and returns Ok(()) after calling purge_data_dir_with_retry(&data_dir).await even when that helper logged a terminal failure; change purge_data_dir_with_retry to return a Result (e.g., Result<(), anyhow::Error> or your crate's error type) and propagate its error: await purge_data_dir_with_retry(&data_dir).await? (or match and return Err) so that the surrounding function does not log success or return Ok(()) when purge failed; update all call sites (the blocks around purge_data_dir_with_retry(&data_dir).await and the duplicate range at 2101-2141) to handle the Result and only log success when purge returns Ok.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src-tauri/src/webview_accounts/mod.rs`:
- Around line 2119-2137: In the purge loop handling remove_dir_all (referencing
remove_dir_all, data_dir, MAX_ATTEMPTS, backoff, schedule_cef_profile_purge),
treat std::io::ErrorKind::NotFound as a success: when Err(err) is returned
during retries, check err.kind() == std::io::ErrorKind::NotFound and if so log a
debug/info message that the directory was already removed and break/return
success instead of counting it as a retry or emitting the final warning;
otherwise continue the existing retry/backoff logic and keep the final warn path
unchanged.
---
Outside diff comments:
In `@app/src-tauri/src/webview_accounts/mod.rs`:
- Around line 2083-2090: The code always logs success and returns Ok(()) after
calling purge_data_dir_with_retry(&data_dir).await even when that helper logged
a terminal failure; change purge_data_dir_with_retry to return a Result (e.g.,
Result<(), anyhow::Error> or your crate's error type) and propagate its error:
await purge_data_dir_with_retry(&data_dir).await? (or match and return Err) so
that the surrounding function does not log success or return Ok(()) when purge
failed; update all call sites (the blocks around
purge_data_dir_with_retry(&data_dir).await and the duplicate range at 2101-2141)
to handle the Result and only log success when purge returns Ok.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3f2fde77-a10d-4199-b7c6-282b19a60900
📒 Files selected for processing (1)
app/src-tauri/src/webview_accounts/mod.rs
* feat(remotion): Ghosty character library with transparent MOV variants (tinyhumansai#1059) Co-authored-by: WOZCODE <contact@withwoz.com> * feat(composio/gmail): sync into memory tree (Slack-parity) (tinyhumansai#1056) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(scheduler-gate): throttle background AI on battery / busy CPU (tinyhumansai#1062) * fix(core,cef): run core in-process and stop orphaning CEF helpers on Cmd+Q (tinyhumansai#1061) * ci: add dedicated staging release workflow (tinyhumansai#1066) * fix(sentry): Rust source context + per-release deploy marker (tinyhumansai#405) (tinyhumansai#1067) * fix(welcome): re-enable OAuth buttons with focus/timeout recovery (tinyhumansai#1049) (tinyhumansai#1069) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore(dependencies): update pnpm-lock.yaml and Cargo.lock for package… (tinyhumansai#1082) * fix(onboarding): personalize welcome agent greeting with user identity (tinyhumansai#1078) * fix(chat): make agent message bubbles fit content width (tinyhumansai#1083) * Feat/dmg checks (tinyhumansai#1084) * fix(linux): Add X11 platform flags to .deb package launcher (tinyhumansai#1087) Co-authored-by: unn-Known1 <unn-known1@users.noreply.github.com> * fix(sentry): auto-send React events; collapse core→tauri for desktop (tinyhumansai#1086) Co-authored-by: Steven Enamakel <enamakel@tinyhumans.ai> * fix(cef): run blank reload guard on the CEF UI thread (tinyhumansai#1092) * fix(app): reload webview instead of restart_app in dev mode (tinyhumansai#1068) (tinyhumansai#1071) * fix(linux): deliver X11 ozone flags via custom .desktop template (tinyhumansai#1091) * fix(webview-accounts): retry data-dir purge so CEF handle race doesn't leak cookies (tinyhumansai#1076) (tinyhumansai#1081) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Steven Enamakel <enamakel@tinyhumans.ai> * fix(webview/slack): media perms + deep-link isolation (tinyhumansai#1074) (tinyhumansai#1080) Co-authored-by: Steven Enamakel <enamakel@tinyhumans.ai> * ci(release): split staging vs production workflows; promote staging tags (tinyhumansai#1094) * Update release-staging.yml (tinyhumansai#1097) * chore(staging): v0.53.5 * chore(staging): v0.53.6 * ci(staging): cut staging from main; add act local-debug helper (tinyhumansai#1099) * chore(staging): v0.53.7 * fix(ci): correct sentry-cli download URL and trap scope (tinyhumansai#1100) * chore(staging): v0.53.8 * feat(chat): forward thread_id to backend for KV cache locality (tinyhumansai#1095) * fix(ci): bump pinned sentry-cli to 3.4.1 (2.34.2 was never published) (tinyhumansai#1102) * chore(staging): v0.53.9 * fix(ci): drop bash trap in upload_sentry_symbols.sh; inline cleanup (tinyhumansai#1103) * chore(staging): v0.53.10 * refactor(session): flatten session_raw/, switch md to YYYY_MM_DD (tinyhumansai#1098) * Add full Composio managed-auth toolkit catalog (tinyhumansai#1093) * ci: add diff-aware 80% coverage gate (Vitest + cargo-llvm-cov) (tinyhumansai#1104) * feat(scripts): pnpm work + pnpm debug for agent-driven workflows (tinyhumansai#1105) * ci: pull pnpm into CI image, drop redundant setup steps (tinyhumansai#1107) * docs: add Cursor Cloud specific instructions to AGENTS.md (tinyhumansai#1106) Co-authored-by: Cursor Agent <cursoragent@cursor.com> * chore(staging): v0.53.11 * docs: surface 80% coverage gate and scripts/debug runners (tinyhumansai#1108) * feat(app): show Composio integrations as sorted icon grid on Skills (tinyhumansai#1109) Co-authored-by: Cursor Agent <cursoragent@cursor.com> * feat(composio): client-side trigger enable/disable toggles (tinyhumansai#1110) * feat(skills): channels grid + integrations card polish; tolerant Composio trigger decode (tinyhumansai#1112) * chore(staging): v0.53.12 * feat(home): early-bird banner + assistant→agent terminology (tinyhumansai#1113) * feat(updater): in-app auto-update with auto-download + restart prompt (tinyhumansai#677) (tinyhumansai#1114) * chore(claude): add ship-and-babysit slash command (tinyhumansai#1115) * feat(home): EarlyBirdyBanner + agent terminology + LinkedIn enrichment model pin (tinyhumansai#1118) * fix(chat): single onboarding thread in sidebar after wizard (tinyhumansai#1116) Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Steven Enamakel <senamakel@users.noreply.github.com> * fix: filter out global namespace from citation chips (tinyhumansai#1124) Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com> Co-authored-by: senamakel-droid <281415773+senamakel-droid@users.noreply.github.com> * feat(nav): enable Memory tab in BottomTabBar (tinyhumansai#1125) * feat(memory): singleton ingestion + status RPC + UI pill (tinyhumansai#1126) * feat(human): mascot tab with viseme-driven lipsync (staging only) (tinyhumansai#1127) * Fix CEF zombie processes on full app close and restart (tinyhumansai#1128) Co-authored-by: senamakel-droid <281415773+senamakel-droid@users.noreply.github.com> Co-authored-by: Steven Enamakel <enamakel@tinyhumans.ai> * Update issue templates for GitHub issue types (tinyhumansai#1146) * feat(human): expand mascot expressions and tighten reply-speech state machine (tinyhumansai#1147) * feat(memory): ingestion pipeline + tree-architecture docs + ops/schemas split (tinyhumansai#1142) * feat(threads): surface live subagent work in parent thread (tinyhumansai#1122) (tinyhumansai#1159) * fix(human): keep mascot mouth animating when TTS ships no viseme data (tinyhumansai#1160) * feat(composio): consume backend markdownFormatted for LLM output (tinyhumansai#1165) * fix(subagent): lazy-register toolkit actions filtered out of fuzzy top-K (tinyhumansai#1162) * feat(memory): user-facing long-term memory window preset (tinyhumansai#1137) (tinyhumansai#1161) * fix(tauri-shell): proactively kill stale openhuman RPC on startup (tinyhumansai#1166) * chore(staging): v0.53.13 * fix(composio): per-action tool consumes backend markdownFormatted (tinyhumansai#1167) * fix(threads): persist selectedThreadId across reloads (tinyhumansai#1168) * feat(memory_tree): switch embed model to bge-m3 (1024-dim, 8K context) (tinyhumansai#1174) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(agent): drop redundant [Memory context] recall injection (tinyhumansai#1173) * chore(memory_tree): drop body-read timeouts on Ollama HTTP calls (tinyhumansai#1171) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(transcript): emit thread_id + fix orchestrator missing cost (tinyhumansai#1169) * fix(composio/gmail): phase out html2md, prefer text/plain MIME part (tinyhumansai#1170) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(tools): markdown output for internal tool results (tinyhumansai#1172) * feat(security): enforce prompt-injection guard before model and tool execution (tinyhumansai#1175) * fix(cef): popup paint dies after first frame — skip blank-page guard for popups (tinyhumansai#1079) (tinyhumansai#1182) Co-authored-by: Steven Enamakel <31011319+senamakel@users.noreply.github.com> * chore(sentry): rename OPENHUMAN_SENTRY_DSN → OPENHUMAN_CORE_SENTRY_DSN (tinyhumansai#1186) * feat(remotion): add yellow mascot character with all animation variants (tinyhumansai#1193) Co-authored-by: Neel Mistry <neelmistry@Neels-MacBook-Pro.local> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor(composio): hide raw connection ID, derive friendly label (tinyhumansai#1153) (tinyhumansai#1185) Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com> * fix(windows): align install.ps1 MSI with per-machine scope (tinyhumansai#913) (tinyhumansai#1187) Co-authored-by: Cursor <cursoragent@cursor.com> * fix(tauri): deterministic CEF teardown on full app close (tinyhumansai#1120) (tinyhumansai#1189) Co-authored-by: Cursor <cursoragent@cursor.com> * fix(composio): cap Gmail HTML body before strip (crash mitigation) (tinyhumansai#1191) Co-authored-by: Cursor <cursoragent@cursor.com> * fix(auth): stop stale chat threads after signup (tinyhumansai#1192) Co-authored-by: Cursor <cursoragent@cursor.com> * feat(sentry): staging-only "Trigger Sentry Test" button (tinyhumansai#1072) (tinyhumansai#1183) * chore(staging): v0.53.14 * chore(staging): v0.53.15 * feat(composio): format trigger slugs into human-readable labels (tinyhumansai#1129) (tinyhumansai#1179) Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com> * fix(ui): hide unsupported permission UI on non-macOS for Screen Intelligence (tinyhumansai#1194) Co-authored-by: Cursor <cursoragent@cursor.com> * chore(tauri-shell): retire embedded Gmail webview-account flow (tinyhumansai#1181) * feat(onboarding): replace welcome-agent bot with react-joyride walkthrough (tinyhumansai#1180) * chore(release): v0.53.16 * fix(threads): preserve selectedThreadId on cold-boot identity hydration (tinyhumansai#1196) * feat(core): version/shutdown/update RPCs + mid-thread integration refresh (tinyhumansai#1195) * fix(mascot): swap to yellow mascot via @remotion/player (tinyhumansai#1200) * feat(memory_tree): cloud-default LLM, queue priority, entity filter, Memory tab UI (tinyhumansai#1198) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Persist turn state + restore conversation history on cold-boot (tinyhumansai#1202) * feat(mascot): floating desktop mascot via native NSPanel + WKWebView (macOS) (tinyhumansai#1203) * fix(memory/tree): emit summary children as Obsidian wikilinks (tinyhumansai#1210) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * feat(tools): coding-harness baseline primitives (tinyhumansai#1205) (tinyhumansai#1208) * docs: add Codex PR checklist for remote agents --------- Co-authored-by: Steven Enamakel <31011319+senamakel@users.noreply.github.com> Co-authored-by: WOZCODE <contact@withwoz.com> Co-authored-by: sanil-23 <sanil@vezures.xyz> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Cyrus Gray <144336577+graycyrus@users.noreply.github.com> Co-authored-by: CodeGhost21 <164498022+CodeGhost21@users.noreply.github.com> Co-authored-by: oxoxDev <164490987+oxoxDev@users.noreply.github.com> Co-authored-by: Mega Mind <146339422+M3gA-Mind@users.noreply.github.com> Co-authored-by: Gaurang Patel <ptelgm.yt@gmail.com> Co-authored-by: unn-Known1 <unn-known1@users.noreply.github.com> Co-authored-by: Steven Enamakel <enamakel@tinyhumans.ai> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Steven Enamakel <senamakel@users.noreply.github.com> Co-authored-by: Steven Enamakel's Droid <enamakel.agent@tinyhumans.ai> Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com> Co-authored-by: senamakel-droid <281415773+senamakel-droid@users.noreply.github.com> Co-authored-by: YellowSnnowmann <167776381+YellowSnnowmann@users.noreply.github.com> Co-authored-by: Neil <neil@maha.xyz> Co-authored-by: Neel Mistry <neelmistry@Neels-MacBook-Pro.local> Co-authored-by: obchain <167975049+obchain@users.noreply.github.com> Co-authored-by: Jwalin Shah <jshah1331@gmail.com>
Summary
webview_account_purgenow retriesremove_dir_allon the per-account data directory with exponential backoff so the CEF / WKWebView handle-release race no longer leaves cookies on disk.tokio::testcases cover the no-op-when-missing path and the happy-path deletion.Problem
After clicking "Remove account" in OpenHuman (Slack / Gmail / Gmeet), re-adding the same provider lands the user already signed in (#1076). Repro is consistent on macOS.
Root cause:
webview_account_purge(app/src-tauri/src/webview_accounts/mod.rs) callswv.close()then immediatelystd::fs::remove_dir_allon the per-account data directory. On macOS, CEF subprocess holds file handles for ~200-500 ms after close returns. The deletion races those handles and fails; the existing code logs a warning and moves on, so cookies survive on disk and the next add lands the previous session.Surfaced as a shared symptom in three independent parity audits (#1016 / #1020 / #1022), so the fix lands at the cross-provider purge primitive instead of per-provider.
Solution
Extract the deletion into
purge_data_dir_with_retry:attempt {n}/{N}and the OS error so a stuck handle is diagnosable from the audit log.schedule_cef_profile_purgeas the cross-launch fallback.The retry budget covers the typical macOS handle-release window with margin. If the worst case exceeds 3 s, the warn log surfaces the leak instead of silently swallowing it.
Submission Checklist
docs/TESTING-STRATEGY.md## Related— no matrix rows touched.docs/TESTING-STRATEGY.md)docs/RELEASE-MANUAL-SMOKE.md.Closes #NNNin the## RelatedsectionImpact
remove_dir_allcall). Failure path adds up to ~3.1 s of waiting before giving up — negligible for a user-initiated logout.Related
schedule_cef_profile_purge(app/src-tauri/src/lib.rs:286) — per-user, queued for next launch; complementary fallback if the retry loop ever times out.Summary by CodeRabbit