Skip to content

fix(macOS): reopen main window after closing it with red X#575

Open
psimaker wants to merge 1 commit intojamiepine:mainfrom
psimaker:fix/macos-reopen-main-window
Open

fix(macOS): reopen main window after closing it with red X#575
psimaker wants to merge 1 commit intojamiepine:mainfrom
psimaker:fix/macos-reopen-main-window

Conversation

@psimaker
Copy link
Copy Markdown

@psimaker psimaker commented Apr 27, 2026

Summary

Fixes a macOS/Tauri window lifecycle issue where closing the main Voicebox window with the red macOS close button leaves the app process running, but clicking the Dock icon does not restore the window.

Problem

On macOS, closing the main window with the red close button could leave Voicebox running without a visible main window.

Local diagnostic after reproducing:

Voicebox process exists
Window count: 0

Fix

On macOS, the red close button for the main window now:

  • prevents the default close
  • runs the existing window-close-requested / window-close-allowed frontend cleanup handshake
  • hides the existing main window instead of destroying it

Dock reopen now honors has_visible_windows and only shows, unminimizes, and focuses the existing main window when no visible windows are present.

This avoids destroying and recreating the renderer, so no frontend readiness handshake or renderer-global ownership migration is needed.

Verification

Automated:

  • git diff --check
  • cargo check
  • bun run typecheck
  • cd tauri && bun run build

Manual macOS QA:

  1. Launch Voicebox with just dev.
  2. Confirm the main window appears.
  3. Click the red macOS close button.
  4. Confirm the app keeps running and the main window is hidden.
  5. Click the Voicebox Dock icon.
  6. Confirm the main window appears again.
  7. Repeat the red-X / Dock-reopen cycle several times.
  8. Rapidly click the red close button after reopening.
  9. Confirm no duplicate cleanup or stuck window state occurs.
  10. Confirm Cmd+Q exits the app cleanly.

Observed result:

macOS close: main window close request intercepted for frontend cleanup
macOS Reopen received — has_visible_windows=false
macOS Reopen: showing/focusing main window
RunEvent::Exit — keep_running=false, has_pid=false
RunEvent::Exit - server will self-terminate via watchdog

Summary by CodeRabbit

macOS Enhancements

  • Improved Window Close Handling
    • Closing the main window now coordinates with the frontend to prevent accidental exits and falls back to hiding after a brief timeout.
  • Reopen Behavior
    • Reopening the app reliably restores and focuses the main window when no visible windows exist.
  • Stability
    • Reduced race conditions during close/reopen sequences for a smoother macOS experience.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 27, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Implements macOS-specific main-window close handling: intercepts CloseRequested for the "main" window to emit window-close-requested, await window-close-allowed (5s timeout), then hide/unminimize/focus the window and set an atomic closing guard; also handles RunEvent::Reopen to restore/focus the main window.

Changes

Cohort / File(s) Summary
macOS Window Lifecycle Management
tauri/src-tauri/src/main.rs
Adds macOS-only interception of CloseRequested for the "main" window that emits window-close-requested, waits for window-close-allowed (5s timeout), hides/unminimizes/focuses the main window, sets an Arc<AtomicBool> closing guard to avoid re-entrancy, and handles RunEvent::Reopen. Keeps existing non-macOS close flow for other windows.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant Frontend
    participant Runtime as Tauri Runtime
    participant Main as main.rs

    rect rgba(100,150,255,0.5)
    Note over User,Main: Window Close Sequence (macOS)
    User->>Frontend: Click close
    Frontend->>Runtime: CloseRequested
    Runtime->>Main: Intercept CloseRequested (label == "main")
    Main->>Frontend: Emit "window-close-requested"
    Main->>Main: Start 5s timeout
    Frontend->>Main: Send "window-close-allowed"
    Main->>Main: Cancel timeout
    Main->>Frontend: Hide window / Unminimize / Focus
    Main->>Main: Set closing = true
    end

    rect rgba(150,200,100,0.5)
    Note over User,Main: Reopen Sequence (macOS)
    User->>Runtime: Click app icon (Reopen)
    Runtime->>Main: RunEvent::Reopen
    Main->>Main: Check visible windows
    alt No visible windows
        Main->>Frontend: Show main window / Unminimize / Focus
    end
    end
Loading

Estimated Code Review Effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐰 I thumped a key, then gave a nod,
"Hold on, don't close the main," I prod.
Five seconds' hush, a frontend cheer,
The window hides — then reappears, my dear.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title accurately describes the main change: implementing macOS-specific window reopen functionality when the main window is closed via the red X button.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share
Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.

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

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
tauri/src-tauri/src/main.rs (1)

71-80: Remove redundant show/unminimize/set_focus calls on freshly-built window to avoid error log spam.

A window built via WebviewWindowBuilder::from_config() automatically honors the config's visible and minimized settings. The main window config here specifies neither (defaulting to visible and not minimized), so the window is already in the desired state after build. The calls on lines 72–80 are no-ops on success but each error is logged via eprintln!, creating noise if macOS quirks cause them to fail (e.g., unminimize on an already-unminimized window or focus-stealing bugs in the tao crate).

The show/unminimize/set_focus triplet on lines 32–40 (existing-window branch) is correct and should stay, since the window's state is unknown. But for the recreated window, drop or demote these calls to debug logging.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tauri/src-tauri/src/main.rs` around lines 71 - 80, The recreated main window
built from WebviewWindowBuilder::from_config() already respects the config
visibility/minimized state, so remove the redundant calls to window.show(),
window.unminimize(), and window.set_focus() in the "recreated window" branch
(the block that prints "macOS Reopen: main window recreated"); alternatively, if
you want to preserve diagnostics, replace those eprintln! error logs with
debug-level logs and only call those methods in the existing-window branch where
state is unknown (keep the window.show()/window.unminimize()/window.set_focus()
calls there).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tauri/src-tauri/src/main.rs`:
- Around line 1438-1449: The current swap-based boolean guard "closing" used in
the WindowEvent::CloseRequested handler is racy and lets a rapid second user
close bypass api.prevent_close(), so replace it with an atomic state machine
(e.g., AtomicU8) with three states: 0=Idle, 1=AwaitingFrontend (prevent user
re-close), 2=ProgrammaticClose (allow through). Update the handler logic around
WindowEvent::CloseRequested to: read the state atomically, if Idle transition to
AwaitingFrontend and call api.prevent_close() to start cleanup; if
AwaitingFrontend ignore subsequent user closes; if ProgrammaticClose allow the
event through and reset to Idle. Ensure the async cleanup task sets the state to
ProgrammaticClose just before calling window.close(), and resets to Idle if
cleanup aborts, so window.close() from the task is not treated as a user click;
keep references to "closing", WindowEvent::CloseRequested, api.prevent_close(),
and window.close() to locate replacements.

---

Nitpick comments:
In `@tauri/src-tauri/src/main.rs`:
- Around line 71-80: The recreated main window built from
WebviewWindowBuilder::from_config() already respects the config
visibility/minimized state, so remove the redundant calls to window.show(),
window.unminimize(), and window.set_focus() in the "recreated window" branch
(the block that prints "macOS Reopen: main window recreated"); alternatively, if
you want to preserve diagnostics, replace those eprintln! error logs with
debug-level logs and only call those methods in the existing-window branch where
state is unknown (keep the window.show()/window.unminimize()/window.set_focus()
calls there).
🪄 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: f2696ac6-70b3-46f5-8f76-83358f7000eb

📥 Commits

Reviewing files that changed from the base of the PR and between b35b909 and 7bd3209.

📒 Files selected for processing (1)
  • tauri/src-tauri/src/main.rs

Comment thread tauri/src-tauri/src/main.rs Outdated
@psimaker psimaker force-pushed the fix/macos-reopen-main-window branch from 7bd3209 to e38fae8 Compare April 27, 2026 22:01
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tauri/src-tauri/src/main.rs`:
- Around line 1477-1481: When the emit fallback fails in the close handler (the
block with app_handle.emit), the code sets close_state to
CLOSE_STATE_PROGRAMMATIC then calls window.close(). If window.close() fails and
the function returns, close_state remains PROGRAMMATIC causing subsequent user
closes to skip frontend cleanup; change the fallback path to restore close_state
to CLOSE_STATE_IDLE on failure: after attempting window.close().ok() (or if it
returns Err), set close_state.store(CLOSE_STATE_IDLE, Ordering::SeqCst) before
returning so the state mirrors the async close path; locate the logic around
app_handle.emit, close_state, CLOSE_STATE_PROGRAMMATIC and CLOSE_STATE_IDLE to
apply this fix.
🪄 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: 1e81227c-bb86-400b-8d8e-26b3d4275ab7

📥 Commits

Reviewing files that changed from the base of the PR and between 7bd3209 and e38fae8.

📒 Files selected for processing (1)
  • tauri/src-tauri/src/main.rs

Comment thread tauri/src-tauri/src/main.rs Outdated
@psimaker psimaker force-pushed the fix/macos-reopen-main-window branch from e38fae8 to 19335ca Compare April 27, 2026 22:08
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tauri/src-tauri/src/main.rs`:
- Around line 60-82: The recreated-window race happens because
show_or_recreate_main_window emits the "window-close-requested" handshake before
the frontend's setupWindowCloseHandler is registered; fix by adding a
frontend-ready ack: have the frontend send a "main-window-ready" event from
setupWindowCloseHandler when it registers its listener, then in
show_or_recreate_main_window (and the on_window_event close path) wait for that
ack (with a short timeout, e.g. a few seconds) before emitting
"window-close-requested"; if the ack times out treat the window as untrusted and
make the stop/keep-running decision in Rust (or default to safe behavior),
ensuring all references to window-close-requested, setupWindowCloseHandler,
show_or_recreate_main_window, and on_window_event are updated to use the new
handshake and timeout.
- Around line 1592-1595: The macOS Reopen handler currently ignores the
has_visible_windows flag and always calls show_or_recreate_main_window(app);
update the RunEvent::Reopen arm to only call show_or_recreate_main_window when
has_visible_windows is false (e.g. if !has_visible_windows {
show_or_recreate_main_window(app); }) so the handler honors the
has_visible_windows contract; adjust the match arm containing RunEvent::Reopen {
has_visible_windows, .. } 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: b094861c-a718-4f5f-8182-b7ab0e09cca4

📥 Commits

Reviewing files that changed from the base of the PR and between e38fae8 and 19335ca.

📒 Files selected for processing (1)
  • tauri/src-tauri/src/main.rs

Comment thread tauri/src-tauri/src/main.rs Outdated
Comment thread tauri/src-tauri/src/main.rs
@psimaker psimaker force-pushed the fix/macos-reopen-main-window branch from 19335ca to beeee0b Compare April 27, 2026 22:35
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tauri/src-tauri/src/main.rs`:
- Around line 1542-1614: The close-state machine is global but the handler runs
for every window, so change the on_window_event CloseRequested handling to only
operate for the main window: at the start of the WindowEvent::CloseRequested
branch check if window.label() == MAIN_WINDOW_LABEL and return early for
non-main windows (i.e., do not read or mutate main_window_lifecycle.close_state
or frontend_ready for other windows). Ensure all places that compare_exchange,
store, call wait_for_main_window_ready, and invoke request_frontend_window_close
(references: main_window_lifecycle, CLOSE_STATE_IDLE,
CLOSE_STATE_AWAITING_FRONTEND, CLOSE_STATE_PROGRAMMATIC, MAIN_WINDOW_LABEL,
wait_for_main_window_ready, request_frontend_window_close) are only reached when
the label matches MAIN_WINDOW_LABEL.

In `@tauri/src/platform/lifecycle.ts`:
- Around line 67-77: Currently registerWindowCloseHandler swallows listen(...)
failures by setting this.windowCloseHandlerSetup = null and returning, causing
the caller to think setup succeeded; instead propagate the failure so setup
rejects. Change registerWindowCloseHandler (the async function that calls listen
and awaits WINDOW_CLOSE_REQUESTED_EVENT) to rethrow the caught error (or throw a
new Error wrapping it) rather than returning, and avoid marking
windowCloseHandlerSetup as success when listen fails; this ensures callers that
await registerWindowCloseHandler observe the failure and Rust receives a correct
readiness signal.
- Around line 91-104: The check using the renderer global
window.__voiceboxServerStartedByApp is unreliable after the "main" window is
recreated; replace that client-side ownership check with an authoritative
backend query: call the existing backend command that reads ServerState (e.g.,
an IPC/Tauri command like a Rust function that returns whether the server was
started/owned by this app) and use its boolean result instead of
window.__voiceboxServerStartedByApp before deciding to call this.stopServer();
keepServer variable use stays the same, but locate the logic around the
window-close handling and update it to await the backend ownership check
(referring to stopServer() and the ServerState-backed command name) so cleanup
runs even after renderer recreation.
🪄 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: ee9a0ef9-0e22-4154-a8d0-761a9cc800bc

📥 Commits

Reviewing files that changed from the base of the PR and between 19335ca and beeee0b.

📒 Files selected for processing (2)
  • tauri/src-tauri/src/main.rs
  • tauri/src/platform/lifecycle.ts

Comment thread tauri/src-tauri/src/main.rs Outdated
Comment thread tauri/src/platform/lifecycle.ts Outdated
Comment on lines 67 to 77
private async registerWindowCloseHandler(): Promise<void> {
try {
// Listen for window close request from Rust
await listen<null>('window-close-requested', async () => {
// Import store here to avoid circular dependency
const { useServerStore } = await import('@/stores/serverStore');
const keepRunning = useServerStore.getState().keepServerRunningOnClose;

// Check if server was started by this app instance
// @ts-expect-error - accessing module-level variable from another module
const serverStartedByApp = window.__voiceboxServerStartedByApp ?? false;

console.log(
'[lifecycle] window-close-requested: keepRunning=%s, serverStartedByApp=%s',
keepRunning,
serverStartedByApp,
);

if (!keepRunning && serverStartedByApp) {
// Stop server before closing (only if we started it)
try {
await this.stopServer();
} catch (error) {
console.error('Failed to stop server on close:', error);
}
}

// Emit event back to Rust to allow close
await emit('window-close-allowed');
await listen<null>(WINDOW_CLOSE_REQUESTED_EVENT, async () => {
await this.handleWindowCloseRequested();
});
} catch (error) {
this.windowCloseHandlerSetup = null;
console.error('Failed to setup window close handler:', error);
return;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Propagate close-listener setup failure instead of resolving successfully.

If listen(...) fails here, setup still resolves and Rust never gets a reliable readiness signal. That can leave the close flow in a permanently degraded state (repeated wait/keep-open behavior).

Proposed fix
   private async registerWindowCloseHandler(): Promise<void> {
     try {
       // Listen for window close request from Rust
       await listen<null>(WINDOW_CLOSE_REQUESTED_EVENT, async () => {
         await this.handleWindowCloseRequested();
       });
     } catch (error) {
       this.windowCloseHandlerSetup = null;
       console.error('Failed to setup window close handler:', error);
-      return;
+      throw error;
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
private async registerWindowCloseHandler(): Promise<void> {
try {
// Listen for window close request from Rust
await listen<null>('window-close-requested', async () => {
// Import store here to avoid circular dependency
const { useServerStore } = await import('@/stores/serverStore');
const keepRunning = useServerStore.getState().keepServerRunningOnClose;
// Check if server was started by this app instance
// @ts-expect-error - accessing module-level variable from another module
const serverStartedByApp = window.__voiceboxServerStartedByApp ?? false;
console.log(
'[lifecycle] window-close-requested: keepRunning=%s, serverStartedByApp=%s',
keepRunning,
serverStartedByApp,
);
if (!keepRunning && serverStartedByApp) {
// Stop server before closing (only if we started it)
try {
await this.stopServer();
} catch (error) {
console.error('Failed to stop server on close:', error);
}
}
// Emit event back to Rust to allow close
await emit('window-close-allowed');
await listen<null>(WINDOW_CLOSE_REQUESTED_EVENT, async () => {
await this.handleWindowCloseRequested();
});
} catch (error) {
this.windowCloseHandlerSetup = null;
console.error('Failed to setup window close handler:', error);
return;
}
private async registerWindowCloseHandler(): Promise<void> {
try {
// Listen for window close request from Rust
await listen<null>(WINDOW_CLOSE_REQUESTED_EVENT, async () => {
await this.handleWindowCloseRequested();
});
} catch (error) {
this.windowCloseHandlerSetup = null;
console.error('Failed to setup window close handler:', error);
throw error;
}
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tauri/src/platform/lifecycle.ts` around lines 67 - 77, Currently
registerWindowCloseHandler swallows listen(...) failures by setting
this.windowCloseHandlerSetup = null and returning, causing the caller to think
setup succeeded; instead propagate the failure so setup rejects. Change
registerWindowCloseHandler (the async function that calls listen and awaits
WINDOW_CLOSE_REQUESTED_EVENT) to rethrow the caught error (or throw a new Error
wrapping it) rather than returning, and avoid marking windowCloseHandlerSetup as
success when listen fails; this ensures callers that await
registerWindowCloseHandler observe the failure and Rust receives a correct
readiness signal.

Comment thread tauri/src/platform/lifecycle.ts Outdated
Comment on lines +91 to +104
// Check if server was started by this app instance
// @ts-expect-error - accessing module-level variable from another module
const serverStartedByApp = window.__voiceboxServerStartedByApp ?? false;

console.log(
'[lifecycle] window-close-requested: keepRunning=%s, serverStartedByApp=%s',
keepRunning,
serverStartedByApp,
);

if (!keepRunning && serverStartedByApp) {
// Stop server before closing (only if we started it)
try {
await this.stopServer();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Use backend-owned state for “server started by app” after window recreation.

window.__voiceboxServerStartedByApp is tied to the renderer context. After recreating "main", this can reset to false, causing close cleanup to skip stopServer() even when this app launched the server.

Proposed direction
-    // `@ts-expect-error` - accessing module-level variable from another module
-    const serverStartedByApp = window.__voiceboxServerStartedByApp ?? false;
+    const serverStartedByApp = await invoke<boolean>('is_server_managed_by_app');

Then back this with a Rust command that reads authoritative server ownership from ServerState (e.g., managed child process presence), not renderer globals.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tauri/src/platform/lifecycle.ts` around lines 91 - 104, The check using the
renderer global window.__voiceboxServerStartedByApp is unreliable after the
"main" window is recreated; replace that client-side ownership check with an
authoritative backend query: call the existing backend command that reads
ServerState (e.g., an IPC/Tauri command like a Rust function that returns
whether the server was started/owned by this app) and use its boolean result
instead of window.__voiceboxServerStartedByApp before deciding to call
this.stopServer(); keepServer variable use stays the same, but locate the logic
around the window-close handling and update it to await the backend ownership
check (referring to stopServer() and the ServerState-backed command name) so
cleanup runs even after renderer recreation.

@psimaker psimaker force-pushed the fix/macos-reopen-main-window branch from beeee0b to 0ee3982 Compare April 27, 2026 22:56
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (1)
tauri/src-tauri/src/main.rs (1)

1475-1479: ⚠️ Potential issue | 🟡 Minor

Reset closing flag if emit-fallback close fails.

If the emit fails and window.close() also fails (or triggers a re-entrant CloseRequested that returns early), the closing flag remains true. Subsequent user-initiated closes will bypass frontend cleanup entirely because line 1464 returns early without calling api.prevent_close().

The macOS main window path (line 68) correctly resets the flag on failure; this path should do the same for consistency.

Proposed fix
                 if let Err(e) = app_handle.emit(WINDOW_CLOSE_REQUESTED_EVENT, ()) {
                     eprintln!("Failed to emit {WINDOW_CLOSE_REQUESTED_EVENT} event: {}", e);
-                    window.close().ok();
+                    if let Err(close_err) = window.close() {
+                        eprintln!("Failed to close window after emit failure: {}", close_err);
+                        closing.store(false, Ordering::SeqCst);
+                    }
                     return;
                 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tauri/src-tauri/src/main.rs` around lines 1475 - 1479, The current close-path
sets the shared closing flag true and then emits WINDOW_CLOSE_REQUESTED_EVENT;
if emit or the fallback window.close() fails the closing flag stays true and
blocks future close flows. Update the closure handling around
app_handle.emit(WINDOW_CLOSE_REQUESTED_EVENT) and the fallback window.close() to
reset the shared closing flag back to false when either emit fails or
window.close() returns Err or otherwise does not complete the frontend
handshake, mirroring the macOS main-window path behavior; ensure you reference
and clear the same flag used by the early-return at the api.prevent_close()
check so subsequent user-initiated closes run the normal cleanup.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@tauri/src-tauri/src/main.rs`:
- Around line 1475-1479: The current close-path sets the shared closing flag
true and then emits WINDOW_CLOSE_REQUESTED_EVENT; if emit or the fallback
window.close() fails the closing flag stays true and blocks future close flows.
Update the closure handling around app_handle.emit(WINDOW_CLOSE_REQUESTED_EVENT)
and the fallback window.close() to reset the shared closing flag back to false
when either emit fails or window.close() returns Err or otherwise does not
complete the frontend handshake, mirroring the macOS main-window path behavior;
ensure you reference and clear the same flag used by the early-return at the
api.prevent_close() check so subsequent user-initiated closes run the normal
cleanup.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8cea5dcc-d798-47da-8ec2-af0892d518c1

📥 Commits

Reviewing files that changed from the base of the PR and between beeee0b and 0ee3982.

📒 Files selected for processing (1)
  • tauri/src-tauri/src/main.rs

@psimaker
Copy link
Copy Markdown
Author

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 27, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (1)
tauri/src-tauri/src/main.rs (1)

1475-1478: ⚠️ Potential issue | 🟠 Major

Reset closing if the fallback window.close() fails.

If this fallback close errors, the window stays open with closing=true, and the next user close bypasses the frontend cleanup path. Mirror the macOS helper here and restore the guard on failure.

🩹 Proposed fix
                     if let Err(e) = app_handle.emit(WINDOW_CLOSE_REQUESTED_EVENT, ()) {
                         eprintln!("Failed to emit {WINDOW_CLOSE_REQUESTED_EVENT} event: {}", e);
-                        window.close().ok();
+                        if let Err(close_err) = window.close() {
+                            eprintln!("Failed to close window after emit failure: {}", close_err);
+                            closing.store(false, Ordering::SeqCst);
+                        }
                         return;
                     }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tauri/src-tauri/src/main.rs` around lines 1475 - 1478, When the fallback
window.close() call fails the code must reset the closing guard so the window
isn't left stuck with closing=true; after attempting the fallback close in the
block that calls app_handle.emit(WINDOW_CLOSE_REQUESTED_EVENT, ()), check the
Result from window.close() and on Err(...) restore the closing flag (the same
guard used around the close flow, e.g. the closing variable/Mutex/Atomic) back
to false and log the error, mirroring the macOS helper behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@tauri/src-tauri/src/main.rs`:
- Around line 1475-1478: When the fallback window.close() call fails the code
must reset the closing guard so the window isn't left stuck with closing=true;
after attempting the fallback close in the block that calls
app_handle.emit(WINDOW_CLOSE_REQUESTED_EVENT, ()), check the Result from
window.close() and on Err(...) restore the closing flag (the same guard used
around the close flow, e.g. the closing variable/Mutex/Atomic) back to false and
log the error, mirroring the macOS helper behavior.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: d214a2f5-f82f-4abc-a278-810d07046877

📥 Commits

Reviewing files that changed from the base of the PR and between beeee0b and 0ee3982.

📒 Files selected for processing (1)
  • tauri/src-tauri/src/main.rs

@psimaker psimaker force-pushed the fix/macos-reopen-main-window branch from 0ee3982 to b80bebe Compare April 28, 2026 06:00
@psimaker
Copy link
Copy Markdown
Author

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 30, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
tauri/src-tauri/src/main.rs (1)

1463-1467: ⚡ Quick win

Rapid double-click can bypass frontend cleanup on non-main windows.

The load/store pattern here differs from the compare_exchange approach used for the macOS main window. When closing.load() returns true, returning without calling api.prevent_close() allows the window to close prematurely—before the async cleanup task completes.

The macOS main window path avoids this by calling hide() (which doesn't trigger another CloseRequested), but this path calls close() which re-enters the handler.

Two fix options:

Option 1: Always prevent re-entry (simpler)
Use destroy() in the async task (bypasses CloseRequested) and always prevent close when the guard is set:

Proposed fix
 if closing.load(Ordering::SeqCst) {
+    api.prevent_close();
     return;
 }

And in the async task (lines 1496, 1500):

-window_for_close.close().ok();
+window_for_close.destroy().ok();

Option 2: State machine (matches macOS main window approach)
Use compare_exchange and track cleanup vs programmatic close states, similar to what was implemented for the main window.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tauri/src-tauri/src/main.rs` around lines 1463 - 1467, The current close
handler uses closing.load()/closing.store() which lets a rapid second
CloseRequested bypass frontend cleanup because the handler returns without
calling api.prevent_close(); change the logic so the handler prevents re-entry:
use an atomic compare_exchange on the closing flag (matching the macOS main
window pattern) or, at minimum, always call api.prevent_close() when closing is
already set to true; then perform the async cleanup task and call
window.destroy() (not close()) at the end to avoid re-triggering CloseRequested;
update the CloseRequested handler references to closing, api.prevent_close(),
hide()/close() paths, the async cleanup task, and use destroy() to finish the
programmatic close.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@tauri/src-tauri/src/main.rs`:
- Around line 1463-1467: The current close handler uses
closing.load()/closing.store() which lets a rapid second CloseRequested bypass
frontend cleanup because the handler returns without calling
api.prevent_close(); change the logic so the handler prevents re-entry: use an
atomic compare_exchange on the closing flag (matching the macOS main window
pattern) or, at minimum, always call api.prevent_close() when closing is already
set to true; then perform the async cleanup task and call window.destroy() (not
close()) at the end to avoid re-triggering CloseRequested; update the
CloseRequested handler references to closing, api.prevent_close(),
hide()/close() paths, the async cleanup task, and use destroy() to finish the
programmatic close.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 7f428272-8c39-4ff2-a5c0-1c6fafb81f94

📥 Commits

Reviewing files that changed from the base of the PR and between 0ee3982 and b80bebe.

📒 Files selected for processing (1)
  • tauri/src-tauri/src/main.rs

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant