fix(tauri): forward Windows local-runtime OAuth callbacks#2469
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThis PR adds Windows-specific named-pipe inter-process communication to forward OAuth and deep-link callbacks from secondary instances to the primary instance, solving a pre-CEF startup delivery problem. It also updates the Windows CI workflow to use pnpm and adds German translations for MCP and subconscious features. ChangesWindows Deep-Link IPC System
German Localization Updates
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
|
@graycyrus @senamakel This is currently the highest-impact user-facing blocker I found in today's issue triage: it addresses the Windows sign-in/deep-link loop reported in #2466 and likely #2462/#2473. Latest effective CI is green, CodeRabbit approved/no actionable comments, and the PR is mergeable. Please prioritize human review/merge if the implementation direction looks acceptable. |
M3gA-Mind
left a comment
M3gA-Mind
left a comment
There was a problem hiding this comment.
Windows OAuth callback — pre-CEF named-pipe bridge
Clean, well-scoped fix. The root cause (pre-CEF mutex guard exiting secondary before Tauri's single-instance/deep-link plugins can run) is correctly diagnosed, and the three-phase solution — secondary forwards via pipe → primary listens → drain on deep-link registration — maps directly to the problem. The fix is Windows-only and properly gated with #![cfg(target_os = "windows")] in the module file plus call sites behind #[cfg(windows)] in lib.rs.
A few things done well: redact_url_for_log strips query/fragment before logging so OAuth tokens never appear in logs or Sentry breadcrumbs; unit tests cover arg filtering, URL redaction, and pipe name stability; the yarn → pnpm CI fix unblocks the Windows workflow for everyone.
Change summary
| Area | Files | Summary |
|---|---|---|
| Tauri shell (Windows) | deep_link_ipc_windows.rs (new, 373 LOC) |
Named-pipe bridge: secondary forwards URLs, primary listens and drains |
| Tauri shell (Windows) | lib.rs |
Wires bridge into the pre-CEF mutex secondary-exit path and setup() |
| Cargo | Cargo.toml |
Adds Win32_Storage_FileSystem, Win32_System_IO, Win32_System_Pipes |
| CI | build-windows.yml |
Corepack + pnpm install --frozen-lockfile |
| i18n | de-3.ts, de-5.ts |
Restores missing German translations for subconscious + MCP server settings |
One minor note inline; no blockers.
| } | ||
| } | ||
|
|
||
| if let Ok(mut queue) = pending_queue().lock() { |
There was a problem hiding this comment.
[minor] Two separate lock acquisitions create a narrow TOCTOU window.
If a URL arrives from the pipe thread after the live_handler lock is released (handler seen as None) but before pending_queue is locked here, drain_pending_urls can fire on the setup thread between those two points — it sets the handler and drains the queue — leaving the URL pushed into an already-drained queue with no subsequent drain scheduled.
The window is startup-only (only open until setup() runs) and realistically only reachable if a second OAuth callback fires while the app is still initialising, so the blast radius is negligible in practice. A straightforward fix is to hold a single combined lock through both operations, or to check the handler again after acquiring the queue lock:
if let Ok(mut queue) = pending_queue().lock() {
// Re-check handler under the queue lock to close the TOCTOU window
if let Ok(guard) = live_handler().lock() {
if let Some(ref handler) = *guard {
handler(url);
return;
}
}
queue.push(url);
}
M3gA-Mind
left a comment
M3gA-Mind
left a comment
There was a problem hiding this comment.
pr-manager: TOCTOU fix below is required before merge. The fix has been committed locally on pr/2469 — apply the patch to your branch and push. All other checks (typecheck, lint, cargo check core + tauri) pass.
| } | ||
| } | ||
|
|
||
| if let Ok(mut queue) = pending_queue().lock() { |
There was a problem hiding this comment.
Fix (applied locally, needs push to PR branch): Addressing the TOCTOU window flagged by @M3gA-Mind.
Acquire pending_queue first, then re-check live_handler under that lock. This closes the window where drain_pending_urls could fire between the two separate lock acquisitions.
fn dispatch_url(url: String) {
if let Ok(mut queue) = pending_queue().lock() {
// Re-check handler under the queue lock to close the TOCTOU window:
// if drain_pending_urls fires between releasing the handler lock and
// acquiring the queue lock, the URL would be pushed into an already-
// drained queue with no subsequent drain scheduled.
if let Ok(guard) = live_handler().lock() {
if let Some(ref handler) = *guard {
handler(url);
return;
}
}
log::debug!(
\"[deep-link-ipc] queued URL before setup: {}\",
redact_url_for_log(&url)
);
queue.push(url);
}
}Please apply this patch to your branch and push.
3 participants
Summary
openhuman://callbacks.Root Cause
The Windows pre-CEF mutex guard exits secondary
OpenHuman.exelaunches before Tauri's single-instance/deep-link plugins can run. Browser OAuth callbacks arrive as a secondary launch withopenhuman://auth?token=..., so the callback URL was dropped while the already-running app only regained focus.User Impact
This should unblock Windows local runtime sign-in when users choose Google or GitHub and the browser callback says it will reopen OpenHuman.
Closes #2462
Closes #2466
Validation
cargo fmt --manifest-path app/src-tauri/Cargo.toml --all -- --checkgit diff --checkcargo metadata --manifest-path app/src-tauri/Cargo.toml --format-version 1 --no-depspnpm i18n:check3f49b679.26261894271, producingwindows-msiandwindows-nsisartifacts.Blocked locally:
cargo test --manifest-path app/src-tauri/Cargo.toml deep_link_ipc_windows --libreached native build scripts but could not complete because this machine is missinglibclang.dllforwhisper-rs-sysandninjaforcef-dll-sys.Summary by CodeRabbit
New Features
Documentation