Skip to content

fix(windows): prevent crash on activate after onboarding wizard closes - #35

Open
espinosacodes wants to merge 1 commit into
TechyCSR:mainfrom
espinosacodes:fix/destroyed-window-crash-on-activate
Open

fix(windows): prevent crash on activate after onboarding wizard closes#35
espinosacodes wants to merge 1 commit into
TechyCSR:mainfrom
espinosacodes:fix/destroyed-window-crash-on-activate

Conversation

@espinosacodes

Copy link
Copy Markdown

Problem

Closing the first-run onboarding wizard and then re-activating the app (clicking the dock icon) crashes the main process:

TypeError: Object has been destroyed
    at /OpenCluely/main.js:1552:20
    at Map.forEach (<anonymous>)
    at ApplicationController.onActivate (/OpenCluely/main.js:1551:29)
    at App.<anonymous> (/OpenCluely/main.js:181:35)

Repro: launch with first-run onboarding (no GEMINI_API_KEY configured), close the wizard, click the dock icon.

Cause

setupWindowEventHandlers() wires the closed handler — the one that prunes the Map via this.windows.delete(type) — by iterating this.windows once, at startup. At that moment the Map holds only main, chat, llmResponse, and settings.

The onboarding wizard is created later, in showOnboarding(), and inserted into that same Map:

onboardingWindow = await this.createWindow('onboarding');
this.windows.set('onboarding', onboardingWindow);   // never gets a 'closed' handler

Since it missed the one-time wiring, its entry outlives the destroyed BrowserWindow. The next activate event iterates the Map and calls isVisible() on the stale entry.

main.js:1551 is the other half: it's an unguarded iteration, whereas window.manager.js consistently guards with isDestroyed() (lines 211, 573, 598, 639, 680, 1012, …).

Fix

  • window.manager.js — attach a closed handler when the onboarding window is created, so it's pruned like every other window. This is the root cause.
  • main.js — guard the onActivate iteration (and the mainWindow check above it) with isDestroyed(), matching the prevailing pattern.

Notes

  • getWindowStats() in window.manager.js has the same unguarded iteration (isVisible() / isFocused() / getPosition()) and would crash the same way once any post-startup window is destroyed. Left out to keep this PR to the reported crash — happy to include it if you'd prefer.
  • The general trap is that any window created after setupWindowEventHandlers() runs inherits this bug. Moving the closed wiring into createWindow() would fix the class rather than the instance; that felt like a larger refactor than this fix warrants, but say the word.

Verification

App boots clean with the change (0 errors through onboarding display). The crashing path itself is GUI-driven (close wizard → dock activate), so it was verified by inspection against the stack trace rather than automated.

The 'closed' handlers that prune this.windows are wired once, in
setupWindowEventHandlers(), over the windows that exist at startup
(main, chat, llmResponse, settings). The onboarding wizard is created
later in showOnboarding() and inserted into the same Map, so it never
receives one — its entry outlives the destroyed BrowserWindow.

The next 'activate' event then iterates the Map and calls isVisible()
on the destroyed window:

  TypeError: Object has been destroyed
      at main.js:1552
      at ApplicationController.onActivate (main.js:1551)

Repro: launch with first-run onboarding, close the wizard, click the
dock icon.

- window.manager.js: attach a 'closed' handler when the onboarding
  window is created, so it is removed from the Map like the rest.
- main.js: guard the onActivate iteration with isDestroyed(), matching
  the pattern already used throughout window.manager.js.
@vercel

vercel Bot commented Jul 31, 2026

Copy link
Copy Markdown

@espinosacodes is attempting to deploy a commit to the csrsoftwares' projects Team on Vercel.

A member of the Team first needs to authorize it.

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