Skip to content

fix(startup): quiet warning noise#1699

Merged
zerob13 merged 2 commits into
devfrom
fix/nan-on-boot
May 28, 2026
Merged

fix(startup): quiet warning noise#1699
zerob13 merged 2 commits into
devfrom
fix/nan-on-boot

Conversation

@zerob13
Copy link
Copy Markdown
Collaborator

@zerob13 zerob13 commented May 28, 2026

Summary

  • normalize missing/invalid lifecycle hook delay values to avoid TimeoutNaNWarning
  • add quiet optional renderer delivery for startup-safe EventBus broadcasts
  • document the issue with SDD artifacts and add focused tests

Tests

  • pnpm exec vitest --config vitest.config.ts run test/main/eventbus/eventbus.test.ts test/main/presenter/lifecyclePresenter/lifecycleDelay.test.ts
  • pnpm run format
  • pnpm run i18n
  • pnpm run lint
  • pnpm run typecheck:node
  • pre-commit typecheck

Summary by CodeRabbit

  • Bug Fixes

    • Eliminated misleading startup warnings when lifecycle settings are omitted or early main-process events occur before any renderer is available.
    • Invalid, missing, negative, or non-finite lifecycle hook delays now default to 0ms and no warning is shown.
  • New Features

    • Renderer delivery of certain notifications is now conditional when a renderer window is not yet present.
  • Tests

    • Added unit tests covering delay normalization and conditional renderer delivery.
  • Documentation

    • Added spec, plan, and task docs describing the startup warning cleanup and acceptance criteria.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 28, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: f26d6142-2ee4-4c83-9e8a-0fd22224b40f

📥 Commits

Reviewing files that changed from the base of the PR and between 149a05c and 10ffb11.

📒 Files selected for processing (2)
  • src/main/eventbus.ts
  • test/main/eventbus/eventbus.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/main/eventbus.ts
  • test/main/eventbus/eventbus.test.ts

📝 Walkthrough

Walkthrough

This PR prevents misleading startup warnings by normalizing lifecycle hook delays to zero for invalid inputs and making EventBus renderer dispatch conditional on window presenter availability, with comprehensive tests for both normalization edge cases and optional dispatch behavior.

Changes

Startup Warning Cleanup

Layer / File(s) Summary
Specification and planning documentation
docs/issues/startup-warning-cleanup/spec.md, docs/issues/startup-warning-cleanup/plan.md, docs/issues/startup-warning-cleanup/tasks.md
Specification defines the goal and acceptance criteria for preventing misleading warnings when lifecycle settings are omitted or when early main-process events occur before renderer windows exist; plan and tasks document the implementation steps including lifecycle delay normalization, optional EventBus dispatch, and configuration notification routing.
Lifecycle hook delay normalization
src/main/presenter/lifecyclePresenter/lifecycleDelay.ts, src/main/presenter/lifecyclePresenter/index.ts, test/main/presenter/lifecyclePresenter/lifecycleDelay.test.ts
New normalizeLifecycleHookDelayMs utility coerces environment variables to non-negative finite milliseconds, returning zero for missing, invalid, negative, or non-finite inputs; integrated into LifecycleManager and tested across missing/empty, invalid/non-finite/negative, and valid delay inputs.
EventBus optional renderer dispatch refactor
src/main/eventbus.ts, test/main/eventbus/eventbus.test.ts
EventBus adds sendToRendererIfAvailable method that conditionally dispatches to renderer and returns boolean indicating success; refactors dispatch logic into dispatchToRenderer helper; send method now silently skips renderer delivery when window presenter is unavailable; tests verify optional dispatch returns false with no warning when presenter is unset and returns true with successful dispatch when present, and verify main-process listeners still receive events without renderer.
Configuration notification optional dispatch
src/main/presenter/configPresenter/index.ts
ConfigPresenter routes session-list notifications through new optional renderer dispatch, allowing startup ACP/session-list updates to complete gracefully without warning when renderer window is not yet initialized.

Sequence Diagram

sequenceDiagram
  participant Caller
  participant EventBus
  participant sendToRendererIfAvailable
  participant windowPresenter
  participant Renderer
  Caller->>EventBus: send(eventName, args)
  EventBus->>sendToRendererIfAvailable: sendToRendererIfAvailable(eventName, args)
  alt windowPresenter exists
    sendToRendererIfAvailable->>windowPresenter: dispatchToRenderer(eventName)
    windowPresenter->>Renderer: deliver event
    sendToRendererIfAvailable-->>EventBus: return true
  else windowPresenter unavailable
    sendToRendererIfAvailable-->>EventBus: return false (silent skip)
  end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

  • ThinkInAIXYZ/deepchat#509: Both PRs touch renderer-dispatch logic in src/main/eventbus.ts involving SendTarget routing and window/default-tab presenter calls, suggesting potential overlap in the default-window dispatch refactoring.

Poem

🐰 A rabbit's ode to silence at startup
When windows don't yet exist to show the way,
We whisper warnings that won't delay the day,
Delays normalized, dispatches made with care,
Early events flow smooth without a scare. 🌅

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'fix(startup): quiet warning noise' is vague and generic, using non-descriptive phrasing that doesn't clearly convey what specific warning issue is being addressed or how it's resolved. Consider a more specific title such as 'fix(startup): normalize lifecycle delay and quiet EventBus warnings' or 'fix(startup): suppress TimeoutNaNWarning and optional renderer delivery' to better reflect the core changes.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
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 docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/nan-on-boot

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.


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 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

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/main/eventbus.ts`:
- Around line 76-82: The routing logic incorrectly lets SendTarget.DEFAULT_TAB
fall through to sendToDefaultWindow when sendToDefaultWindow exists; update the
branch so SendTarget.DEFAULT_WINDOW always calls
windowPresenter.sendToDefaultWindow(eventName, true, ...args) and
SendTarget.DEFAULT_TAB always calls windowPresenter.sendToDefaultTab(eventName,
true, ...args), using explicit checks on the SendTarget value (or separate case
blocks) and still guarding each call with typeof
windowPresenter.sendToDefaultWindow === 'function' / typeof
windowPresenter.sendToDefaultTab === 'function' as appropriate to avoid runtime
errors.
🪄 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: 10616021-4d70-4a89-8646-d487c8ede5bb

📥 Commits

Reviewing files that changed from the base of the PR and between d780d1a and 149a05c.

📒 Files selected for processing (9)
  • docs/issues/startup-warning-cleanup/plan.md
  • docs/issues/startup-warning-cleanup/spec.md
  • docs/issues/startup-warning-cleanup/tasks.md
  • src/main/eventbus.ts
  • src/main/presenter/configPresenter/index.ts
  • src/main/presenter/lifecyclePresenter/index.ts
  • src/main/presenter/lifecyclePresenter/lifecycleDelay.ts
  • test/main/eventbus/eventbus.test.ts
  • test/main/presenter/lifecyclePresenter/lifecycleDelay.test.ts

Comment thread src/main/eventbus.ts Outdated
@zerob13
Copy link
Copy Markdown
Collaborator Author

zerob13 commented May 28, 2026

Addressed the CodeRabbit routing comment in 10ffb11: DEFAULT_WINDOW and DEFAULT_TAB now use separate EventBus branches, and DEFAULT_TAB is covered by a regression test that asserts sendToDefaultTab is used without calling sendToDefaultWindow.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 28, 2026

Actionable comments posted: 0

@zerob13 zerob13 merged commit a1a9523 into dev May 28, 2026
4 checks passed
@zhangmo8 zhangmo8 deleted the fix/nan-on-boot branch May 29, 2026 05:44
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