Skip to content

Refactor Electron startup boot sequence#280

Open
kilbot wants to merge 1 commit into
nextfrom
refactor/explicit-boot-order
Open

Refactor Electron startup boot sequence#280
kilbot wants to merge 1 commit into
nextfrom
refactor/explicit-boot-order

Conversation

@kilbot

@kilbot kilbot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Introduces src/main/boot.ts with an explicit, dependency-injected boot plan and AppContext for startup wiring.
  • Replaces the inline app.whenReady().then(...) chain with boot(bootDeps) while preserving the existing phase order and dev-only protocol handling.
  • Reworks the auto-updater so the real main window is injected after window creation instead of captured as null at module import time.
  • Adds a ts-node boot smoke test and wires it into pnpm test.

Design decisions (preserve through rebases)

  • bootPlan(deps) is exported as ordered data — startup ordering is now inspectable/testable without launching Electron.
  • getMainWindow() remains as a shim — late event-time lookups like power resume stay behavior-preserving while constructor-time ambient grabs are removed.
  • The exported updater remains a stable menu-facing handle — menu call sites keep working while the real AutoUpdater instance is configured post-window.

Test plan

  • pnpm --config.verify-deps-before-run=false run ts:check
  • pnpm --config.verify-deps-before-run=false run lint (exits 0; existing warnings remain)
  • pnpm --config.verify-deps-before-run=false run test:boot
  • pnpm --config.verify-deps-before-run=false test
  • Dev smoke: pnpm --config.verify-deps-before-run=false run dev reached Electron startup, Starting app, auth handler initialization, development updater no-op, renderer bundle load, and hydration logs. API session could not visually click the menu/dialog.
  • Manually verify the main window is visible, menu is present, and “Check for updates” opens a dialog parented to the main window.
  • Packaged build sanity after renderer dist/ is present. In this worktree, pnpm --config.verify-deps-before-run=false run package reached Forge finalization but failed with ENOENT ... lstat .../dist because the renderer dist/ directory was absent.

🤖 Generated with Claude Code

@coderabbitai

coderabbitai Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@kilbot, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 54 minutes and 40 seconds. Learn how PR review limits work.

To continue reviewing without waiting, enable usage-based billing in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository: wcpos/coderabbit/.coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: aff39c30-aab5-4f9d-915b-a45356a1fc83

📥 Commits

Reviewing files that changed from the base of the PR and between 5851f72 and 4426f45.

📒 Files selected for processing (6)
  • package.json
  • src/index.ts
  • src/main/boot.test.ts
  • src/main/boot.ts
  • src/main/update.ts
  • src/main/window.ts
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/explicit-boot-order

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

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4426f45681

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/main/update.ts
constructor(mainWindow: BrowserWindow) {
this.targetPath = '';
this.mainWindow = getMainWindow();
this.mainWindow = mainWindow;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Clear stale updater window after close

On macOS the app intentionally stays running after window-all-closed; with this assignment the updater keeps a reference to the first BrowserWindow even after it has emitted closed. If the user closes the last window and then uses the still-present app menu to check for updates, or the hourly update interval finds one before the app is reactivated, manualCheckForUpdates/confirmUpdateDialog pass a destroyed window into dialog.showMessageBox, so the update UI can fail instead of showing an unparented dialog. Clear or validate the stored window on close before using it.

Useful? React with 👍 / 👎.

@kilbot kilbot changed the base branch from main to next June 17, 2026 22:01
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