fix(desktop): gate last-window quit until startup settles by santhiprakash · Pull Request #2287 · milind-soni/OpenMausBot

santhiprakash · 2026-10-04T11:04:59Z

What changed

window-all-closed no longer quits the app while the boot chain is still running. The quit decision now lives in electron/window-all-closed.mjs, gated on startup having settled; when the boot promise settles it re-checks the window set once, so a boot that genuinely ends with no window still exits instead of idling headless. Behavior after startup and on macOS is unchanged.

Why

Refs #2028 — the AppImage exits silently ~10.6s after launch under a Hyprland/systemd-user session but runs fine from SSH.

The startup splash is the app's only window while boot runs. e169719b ("recover from a stalled loading window") added a render-process-gone handler and a 10s fallback that dispose() the splash so a stalled renderer can't hold boot — but dispose() calls splash.destroy(), which empties the window set, fires window-all-closed, and the unconditional app.quit() at electron/main.mjs turns the recovery into a silent exit ~10.5s in: no window ever showed, the server child was never forked (8799/8800 never come up), and nothing reaches stdout/stderr because startup logging goes to the log file. That matches the reporter's ~10.6s wall-clock / ~1s CPU signature on a session where the splash renderer stalls before ready-to-show.

With this change the drain mid-boot is survivable exactly as the recovery intended: boot continues, the server forks, and the main window (or the bounded error page) still gets delivered.

How it was verified

node --test electron/window-all-closed.node-test.mjs — 5/5 pass, incl. the Linux AppImage exits silently after ~10s when started by the desktop/systemd-user, but stays up when started from an SSH session #2028 sequence (drain mid-boot → boot creates the main window → settle keeps the app up → last close quits)
Sabotage run: removing the startupSettled gate fails the new tests; restored they pass
pnpm test:electron — 555 pass / 0 fail / 5 skipped (560 total)
pnpm check:electron — 148 modules syntax-checked
pnpm typecheck — clean; oxlint --deny-warnings clean
Pre-push hook (lint + tsc -b + i18n:check) — green

Screenshots (UI changes)

No UI change — desktop lifecycle only.

Checklist

pnpm typecheck and pnpm test pass locally (pnpm test:electron; touched surface is the Electron shell)
Server behavior changes come with tests — n/a, desktop lifecycle; regression covered by electron/window-all-closed.node-test.mjs
No dist-server/ edits (it's build output)
macOS-only code is platform-gated; no shell: true / cmd.exe string-building — darwin keeps never-quitting on the drain
No secrets in logs, responses, events, or argv

Notes

The remaining unknown is why the splash renderer stalls in that session — the reporter's portal/D-Bus/keyring guess is plausible and is Chromium-side. Post-fix the app stays alive and boot completes, so if a slow credential-store read ever stalls, it now surfaces in server.log and via ELECTRON_ENABLE_LOGGING instead of a silent exit. No safeStorage path was changed.
One-condition gate; no sibling call sites (the only window-all-closed registration).

AI-assisted. I wrote and verified this change.

Summary by CodeRabbit

Bug Fixes
- Fixed app shutdown behavior when all windows close during startup. The app now waits for startup to finish and confirms no windows remain before quitting on non-macOS platforms. If startup recreates a window, the app stays open until that window is closed. On macOS, closing all windows continues to leave the app running.

- Problem: the startup splash is the only window during boot, and its recovery timers destroy it when the loading renderer stalls; the empty window set then fires window-all-closed and the app quits silently ~10s after launch, before the server child forks (issue milind-soni#2028). - Fix: hold the window-all-closed quit until the boot chain settles, and re-check the drained window set once at settle so a boot that ends windowless still exits instead of idling. - Verification: node --test electron/window-all-closed.node-test.mjs (5/5, sabotage run with the gate removed fails them); pnpm test:electron 555/560 pass 0 fail; pnpm check:electron 148 modules; pnpm typecheck clean.

vercel · 2026-10-04T11:05:03Z

@santhiprakash is attempting to deploy a commit to the SupaMaus Team on Vercel.

A member of the Team first needs to authorize it.

coderabbitai · 2026-10-04T11:06:13Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults
Review profile: CHILL
Plan: Advanced
Run ID: 2c7215b3-6826-4d24-ad12-3f162084d31c

📥 Commits

Reviewing files that changed from the base of the PR and between 616e2ce and d94a45a.

📒 Files selected for processing (1)

electron/main.mjs

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

📝 Walkthrough

Walkthrough

The main process now delegates last-window quit decisions to a coordinator. The coordinator checks whether startup has settled, whether the platform is macOS, and whether any windows remain. Node tests cover these conditions.

Changes

Window lifecycle

Layer / File(s)	Summary
Startup-aware quit policy `electron/window-all-closed.mjs`, `electron/window-all-closed.node-test.mjs`	The helper checks startup state, platform, and the current window list before quitting. Tests cover startup-time closes, live and recreated windows, and macOS behavior.
Main-process integration `electron/main.mjs`	The main process creates the coordinator with the app and current BrowserWindow list instead of directly quitting in its `window-all-closed` handler. Startup completion calls `settleStartup()`.

Priority: ➖ Normal

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

Change: Bug fix

Suggested reviewers: milind-soni

Merge Risk: ⚪ Minimal · up to d94a4

If the last window closes during startup, the app waits for startup to settle before applying its platform-specific quit behavior. No actionable merge-blocking risk is established.

Architecture Summary

Architecture risk: 🟡 Medium · up to 235cd

The change affects 1 system.

Changed systems: electron

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

observed — electron (service) was modified; 3 changed files map to changed impact.

Before / after behavior

observed — Modified behavior in electron/main.mjs: Imports createAllWindowsClosedQuit for the main-process window-close lifecycle.
observed — Modified behavior in electron/main.mjs: After startup settles, constructs the all-windows-closed quit coordinator with the app and current BrowserWindow list. Removes the previous handler that called app.quit() whenever all windows closed outside macOS; the lifecycle decision is now delegated to the coordinator.
observed — Modified behavior in electron/window-all-closed.mjs: Adds an Electron-free quit-policy helper. The window-all-closed event quits only after startup settles, on non-darwin platforms, and when allWindows() returns no windows. settleStartup() marks startup settled and rechecks those conditions, including when the window-close event occurred before startup settled.
observed — Modified behavior in electron/window-all-closed.node-test.mjs: Adds imports for Node assertions and tests, and for createAllWindowsClosedQuit.

Reliability and maintainability

inferred — Risk-relevant change factors for electron: blast_radius_2; direct_dependents_2

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 3 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: delaying the last-window quit decision until startup settles.
Description check	✅ Passed	The description includes all required sections, explains the change and its motivation, reports verification results, and completes the checklist. It also states that no UI changes require screenshots…
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.

Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

🛠️ Fix failing CI checks 💡

Commit to this branch
Create a new PR

🧪 Generate unit tests (beta)

Create a new PR

Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

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

Merge branch 'main' into fix/2028-appimage-dbus-wait

616e2ce

milind-soni enabled auto-merge (squash) October 6, 2026 09:28

Merge branch 'main' into fix/2028-appimage-dbus-wait

d94a45a

milind-soni merged commit 5ca3b8d into milind-soni:main Oct 7, 2026
23 of 24 checks passed

Navigation Menu

Repository navigation

fix(desktop): gate last-window quit until startup settles - #2287

fix(desktop): gate last-window quit until startup settles#2287
milind-soni merged 3 commits into
milind-soni:mainfrom
santhiprakash:fix/2028-appimage-dbus-wait

santhiprakash commented Oct 4, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

vercel Bot commented Oct 4, 2026

Uh oh!

coderabbitai Bot commented Oct 4, 2026 •

edited

Loading

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

santhiprakash commented Oct 4, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changed

Why

How it was verified

Screenshots (UI changes)

Checklist

Notes

Summary by CodeRabbit

Uh oh!

vercel Bot commented Oct 4, 2026

Uh oh!

coderabbitai Bot commented Oct 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Architecture Summary

❌ Failed checks (1 warning)

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

santhiprakash commented Oct 4, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Oct 4, 2026 •

edited

Loading