Skip to content

fix: defer session permission handlers until app ready (#529) - #532

Merged
lacymorrow merged 2 commits into
mainfrom
lac-3530/defer-session-handlers
Aug 26, 2026
Merged

lacymorrow merged 2 commits into
mainfrom
lac-3530/defer-session-handlers

Conversation

@lacymorrow

@lacymorrow lacymorrow commented Aug 26, 2026

Copy link
Copy Markdown
Owner

Summary

Fixes the second startup crash reported in #529 (joncl's report): on some machines, 3.4.1/3.4.4/3.4.5 launch as a single orphaned process with no window and no tray icon, with TypeError: Session can only be received when app is ready in main.log.

Root cause: src/main.js calls register.appEvents() before await app.whenReady(), and appEvents() immediately touched session.defaultSession to install the deny-all permission handlers added in #493. Electron throws if defaultSession is accessed before the ready event, killing the main process before any UI exists. Whether it crashes is a startup-timing race (module load + init vs. Electron's ready event), which is why it reproduces consistently on some machines and never on others.

Fix: the handlers now live in src/main/session-permissions.js, which always defers installation behind app.whenReady(). Handler behavior is unchanged (deny all permission requests/checks); they are installed before any window is created (windows are created 500ms after ready).

Paperclip issue: LAC-3530 (follow-up to LAC-3424 / v3.4.5).

Testing

  • Regression test (test/session-permissions.spec.js): a minimal fake Electron reproduces the real semantics — defaultSession throws until ready. The main test fails on the old synchronous installation (verified red before the fix) and asserts the handlers install after ready and deny all permissions. A guard test keeps direct defaultSession access from being reintroduced anywhere else in src/ (the shared source-tree walker moved into test/helpers.js, reused by config.spec.js).
  • npx playwright test test/session-permissions.spec.js test/config.spec.js — 4/4 pass.
  • Booted the app locally with the fix: clean startup, App ready, window created, no session errors after 12s.
  • npm run lint clean (0 errors) on changed files.
  • Note: the full Playwright app-launch suite times out in this environment on unmodified main too (10s beforeAll hook budget) — pre-existing, unrelated.

🤖 Generated with Claude Code

lacymorrow and others added 2 commits August 26, 2026 17:44
register.appEvents() runs before app.whenReady() and accessed
session.defaultSession to install the deny-all permission handlers
from #493. Electron throws "Session can only be received when app is
ready" on pre-ready access, crashing the main process before any
window or tray exists on machines that lose the startup race —
reported on Windows for 3.4.1/3.4.4/3.4.5.

Move the handlers into src/main/session-permissions.js, which always
defers installation behind app.whenReady(). Regression test mimics
Electron's pre-ready throw semantics and fails on the old synchronous
installation; a guard test keeps direct defaultSession access from
being reintroduced elsewhere in src/.

Fixes the follow-up crash reported in #529 (joncl's report).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Hoist collectSourceFiles/SRC_DIR from config.spec.js into test/helpers.js
and reuse in both guard specs; inline the defaultSession getter, drop
unused fake-app scaffolding, and merge the two overlapping regression
assertions into one test.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@lacymorrow
lacymorrow merged commit 5ccbf06 into main Aug 26, 2026
4 checks passed
@lacymorrow
lacymorrow deleted the lac-3530/defer-session-handlers branch August 26, 2026 21:54
lacymorrow added a commit that referenced this pull request Aug 26, 2026
Version bump for 3.4.6 patch release. Ships fix from #532 (LAC-3530): startup race where session.defaultSession was accessed before app ready.
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