fix(tauri): hide instead of destroy main window on Windows close (OPENHUMAN-TAURI-2X)#1548
Conversation
…NHUMAN-TAURI-2X)
Extend the macOS-only CloseRequested→hide handler at lib.rs to also
include Windows. Without this, the OS destroys the webview when the
user clicks the close button; subsequent tray-icon clicks then call
`show_main_window` which calls `get_webview_window("main")`, returns
None, and emits the warn log that flooded Sentry as OPENHUMAN-TAURI-2X
(21 events, Windows-only).
Linux is left out: `setup_tray` early-returns on Linux because tray
creation panics inside GTK during packaged runs (existing behavior),
so hide-on-close there would strand the user with no way back. The
Linux close path keeps the standard destroy-on-close behavior.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
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 (1)
📝 WalkthroughWalkthroughThe Tauri app event loop now expands close event interception from macOS-only to both macOS and Windows, preventing the webview from being destroyed on window close and enabling tray-based reopen behavior on Windows. Linux remains unchanged. ChangesTray Reopen on Windows
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src-tauri/src/lib.rs`:
- Around line 2090-2093: The Windows compile error is caused because WindowEvent
is only imported behind a macOS cfg while the RunEvent arm references
WindowEvent::CloseRequested for both macOS and Windows; fix it by ensuring
WindowEvent is available for that cfg path—either move or duplicate the use
tauri::WindowEvent import so it’s compiled for cfg(any(target_os = "macos",
target_os = "windows")), or change the match arm to reference the
fully-qualified tauri::WindowEvent::CloseRequested; update the RunEvent match
arm that currently mentions WindowEvent::CloseRequested accordingly.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d3675324-4113-4ac4-83ac-9f7ffcad9630
📒 Files selected for processing (1)
app/src-tauri/src/lib.rs
…ows too (OPENHUMAN-TAURI-2X) Per CodeRabbit critical review on PR tinyhumansai#1548: the prior commit extended the `RunEvent::WindowEvent { … }` match arm to also fire on Windows, but the `use tauri::WindowEvent;` line was left at `#[cfg(target_os = "macos")]`. On Windows builds the match arm references `WindowEvent::CloseRequested` without the symbol in scope, which is a hard compile error. Extend the import cfg to match the arm cfg. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
graycyrus
left a comment
There was a problem hiding this comment.
Review — fix(tauri): hide instead of destroy main window on Windows close
Overall: Clean, well-scoped fix. The cfg widening is mechanically correct, Linux exclusion is justified, and the existing macOS handler body is correct for Windows too. Two minor items and one question below.
Verified ✅
#[cfg]on import and handler match — no compile-time mismatch- Linux exclusion intentional, documented, consistent with
setup_trayearly-return - No new JS injection, deps, secrets, or unwraps
- Existing
log::info!satisfies debug logging requirement show_main_windowis unconditional — correctly finds hidden window on tray click
| #[cfg(any(target_os = "macos", target_os = "windows"))] | ||
| RunEvent::WindowEvent { | ||
| label, | ||
| event: WindowEvent::CloseRequested { api, .. }, |
There was a problem hiding this comment.
[minor] With hide-on-close now active on Windows, the only way to fully quit is the tray's "Quit" menu item. Worth confirming that the tray quit path on Windows:
- Calls
app.exit(0)(or equivalent) — not justwindow.close() - Cleanly terminates the
openhumansidecar process (no orphaned core process after "quit")
If the tray quit item already calls app.exit(), this is a non-issue. A brief note confirming manual Windows testing would close the loop.
Not a code defect in the diff — the behavioral contract just changed for Windows users (X no longer exits the process), so the quit path becomes the sole shutdown vector.
| // None on the next tray-click and surfaces as | ||
| // `[tray] failed to show main window from tray click: main window | ||
| // not found` (OPENHUMAN-TAURI-2X — 21 events, Windows only). | ||
| // Linux is left out: `setup_tray` early-returns on Linux because |
There was a problem hiding this comment.
[nitpick] The parenthetical (OPENHUMAN-TAURI-2X — 21 events, Windows only) embeds a point-in-time event count that will go stale. Consider simplifying to (OPENHUMAN-TAURI-2X, Windows-only) — the issue tracker has the live count.
Summary
[tray] failed to show main window from tray click: main window not found.setup_trayearly-returns on Linux because tray creation panics in GTK during packaged runs.Problem
Pre-fix: clicking the X close button on Windows destroys the main webview window. Subsequent tray-icon clicks call
show_main_window → app.get_webview_window("main"), which returnsNone, surfacing the warn-level log[tray] failed to show main window from tray click: main window not found(app/src-tauri/src/lib.rs:939).That log is captured by Sentry and shows up as OPENHUMAN-TAURI-2X — ~21 events from the China + Thailand Windows users observed in the last 72h. More importantly, the user loses the only re-entry point into the app: tray click is a no-op.
The macOS path already does the right thing (
app/src-tauri/src/lib.rs:2081-2094—CloseRequested → prevent_close → hide), but the#[cfg(target_os = "macos")]arm excluded Windows. macOS users got the canonical "close-button-hides" UX that pairs with the tray icon; Windows users got the standard destroy-on-close that breaks the tray-icon re-entry path.Solution
Single-line cfg extension. The CloseRequested handler at
app/src-tauri/src/lib.rs:2081now matches#[cfg(any(target_os = "macos", target_os = "windows"))]. Same body —api.prevent_close()thenwindow.hide(). Comment expanded to explain why Linux is intentionally excluded.Linux exclusion preserved:
setup_trayreturns early on Linux today (GTK packaged-run panic, see existing code atapp/src-tauri/src/lib.rs:861-867). Hide-on-close without a tray would strand the Linux user with no way to surface the window again, so the standard destroy-on-close behavior is the right default there.The tray-click recovery path (
show_main_windowatapp/src-tauri/src/lib.rs:825-859) is unchanged — once the window is merely hidden rather than destroyed,get_webview_window("main").show().unminimize().set_focus()works exactly as on macOS.Submission Checklist
N/A: pure cfg-attribute extension; the macOS arm is already covered by the manual smoke checklist and there is no Rust-side unit harness forRunEvent::WindowEventhandlers inapp/src-tauri/`. Manual smoke step (Windows close → tray click → window reappears) is the canonical verification.N/A: behavioral parity with the already-shipped macOS branch; the touched lines are an existing handler getting an additional target platform.N/A: no new user-visible feature row.## Related—N/A: behaviour-only platform parity fix.N/A: no new smoke step (this restores the existing tray-click recovery path on Windows; verify on the next Windows release build per the standard tray smoke step).Closesin the## RelatedsectionImpact
RunEventhandler change.Quititem (already present atapp/src-tauri/src/lib.rs:924-927) orFile → Quitif present. This matches the established macOS pattern and matches the typical Win11 tray-app convention (Spotify / Slack / Discord all do this).app/src-tauri/src/lib.rs:939stops firing once the window survives close.Related
AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
fix/2x-tray-window-missingValidation Run
pnpm --filter openhuman-app format:check— N/A: noapp/src/changespnpm typecheck— N/A: no TS changesN/A: no Rust-side test harness for RunEvent::WindowEvent handlers — manual smoke is the verification path.cargo check --manifest-path app/src-tauri/Cargo.tomlclean (2m22s, 26 pre-existing warnings unrelated to this change)app/src-tauri/Cargo.tomlbuild — cleanValidation Blocked
command:N/Aerror:N/Aimpact:N/ABehavior Changes
Parity Contract
tray_show_window,tray_quit) unchanged;show_main_windowunchanged; Linux unchanged.Duplicate / Superseded PR Handling
Summary by CodeRabbit