fix: stabilize Windows-flaky tests (lifecycle teardown race + egg-bin timeouts)

[killagu]

Fixes two independent sources of Windows CI test flakiness on next.

1. fix(core): teardown close/load race (the @eggjs/session flake)

Symptom

Windows CI flakily failed @eggjs/session test/app/middleware/session.test.ts with app has been closed / Can't find viewEngine unhandled rejections (e.g. CI run 27204947537). The failing assertion points at an unrelated file — the signature of a process-level unhandled rejection leaking across files under vitest isolate: false (continuation of #5964).

Root cause

mm.app() loads an app and an agent, and load() runs as a registerBeforeStart hook on process.nextTick, so it can still be in flight when close() runs (slow Windows fs widens the race). Lifecycle.close() does not wait for the in-flight load, so it flips #isClosed while load() is still running. The still-loading code then reaches Lifecycle.registerBeforeClose() — directly in egg.ts load(), or lazily via coreLogger → createLoggers() from dumpTiming / _unhandledRejectionHandler — which assert-threw app has been closed. Inside an async load / rejection handler that becomes a process-level unhandled rejection, which isolate: false attributes to whatever test file is currently running.

Fix

• Lifecycle.registerBeforeClose() now refuses (returns false, no throw) when the app is closing or closed instead of asserting. The guard covers close in progress too (new #isClosing flag set at the top of close(), before the close-callback snapshot is taken), so a hook registered mid-close is not silently stranded.
• EggApplicationCore.load() checks that return value: when registration is refused by a teardown race, it cleans up the resources it already created — the unhandledRejection listener, the messenger (IPC listeners) and any lazily-created loggers (file descriptors) — so none leak across files, then returns without loading a torn-down app.
• New Lifecycle.isClosed / isClosing getters + regression tests (after-close and during-close).

2. test(egg-bin): Windows CI timeouts

Test bin (windows-latest, 24) flakily timed out (every failure was Test timed out in 60000ms, no assertion failures). The egg-bin command tests spawn child egg-bin processes (vitest-in-vitest) that run 50s+ each on Windows; under full parallelism + coverage they tip over the 60s timeout. This reproduces on plain next (run 27863137812), independent of fix #1.

Mirror the root config's Windows handling in tools/egg-bin/vitest.config.ts: on Windows CI, raise testTimeout/hookTimeout to 120s and cap maxWorkers to 2 so fewer child-process trees compete. No effect off Windows.

Test evidence

• packages/core full suite (incl. new lifecycle regression tests) — 476 pass; typecheck + lint + format clean.
• plugins/session/.../session.test.ts — pass.
• tools/egg-bin/test/commands/dev.test.ts — pass locally (≈43s for one file, confirming the subprocess slowness).
• CI after fix Opensource version of egg-bin #1: the main Test matrix incl. Test (windows-latest, 22/24) passed — only the pre-existing egg-bin Windows timeout remained, which fix egg-mock #2 targets.

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features
  • Added lifecycle state visibility via isClosed and isClosing; registerBeforeClose now returns success/failure.
• Bug Fixes
  • Prevent late close-hook registration during in-flight/finished shutdown, avoiding stranded hooks.
  • Improved load/close race safety by short-circuiting teardown when close-hook registration can’t be accepted.
• Tests
  • Added coverage for close-in-flight and post-close hook registration races.
• Documentation
  • Updated guidance for vitest isolate:false state leak scenarios.
• Chores
  • Tuned Vitest timeouts/worker limits for Windows CI and split a slow CLI test to reduce flakiness.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a3bc895b-5683-4cf3-9889-d96099d7df38

📥 Commits

Reviewing files that changed from the base of the PR and between 6d94be5 and 85cbdf6.

📒 Files selected for processing (1)

• packages/egg/src/lib/egg.ts

---

📝 Walkthrough

Walkthrough

Adds isClosing and isClosed getters to expose Lifecycle state. Changes registerBeforeClose to return false instead of throwing when the lifecycle is already closing or closed. Sets #isClosing = true at the start of close() to refuse late hook registrations. Adds regression tests for post-close and during-close registration races. Guards EggApplicationCore to clean up resources immediately when registerBeforeClose fails and refactors timeout management. Updates wiki documentation describing the teardown/load race fix. Configures vitest Windows CI test timeouts, worker limits, and splits a slow sequential test to reduce contention.

Changes

Lifecycle teardown race fix and test hardening

Layer / File(s)	Summary
Lifecycle state flags and getters packages/core/src/lifecycle.ts	Introduces a private #isClosing flag, initializes it to false in the constructor, and adds public isClosed and isClosing getters to expose the internal state.
registerBeforeClose race guard and close() timing packages/core/src/lifecycle.ts	Changes registerBeforeClose to return boolean: returns false (and logs) when #isClosing or #isClosed is already true, else registers the hook and returns true. Also sets #isClosing = true at the start of close() before snapshotting hooks, ensuring registrations during in-flight close are refused.
Regression tests packages/core/test/lifecycle.test.ts	Adds two async test cases: one verifying registerBeforeClose returns false after close() completes without throwing, and one verifying registration during an in-flight close() is refused and does not leave a stranded hook.
EggApplicationCore teardown on registration failure packages/egg/src/lib/egg.ts	Captures the return value of registerBeforeClose(). When registration fails, immediately removes the unhandledRejection listener, closes the messenger, closes any lazily-created loggers, clears the start-timeout timer, and returns early to prevent resource leaks in shutdown races. Refactors timeout management to use an instance field for cleaner teardown.
Wiki documentation wiki/concepts/vitest-isolate-false-state-leaks.md, wiki/index.md, wiki/log.md	Extends source files list in the concept doc, adds root cause entry 4 describing the teardown/in-flight load race and the implemented fixes, updates the index bullet text, and adds a 2026-06-20 log entry.
Windows CI test contention reduction tools/egg-bin/vitest.config.ts, tools/egg-bin/test/my-egg-bin.test.ts	Detects Windows CI environment and adjusts testTimeout, hookTimeout to 120000 ms, and sets maxWorkers: 2 only on Windows CI. Splits a combined nsp test into two separate async test cases with comments explaining per-fork isolation to prevent slow startup timeouts.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• eggjs/egg#5904: Modifies packages/core/src/lifecycle.ts close hook execution control flow, overlapping with the same lifecycle close and hook handling logic changed in this PR.

Suggested reviewers

• jerryliang64
• gxkl
• akitaSummer

Poem

🐇 A rabbit checks the closing door—
Is it locked? Did we close before?
Late arrivals get "false" returned,
No stranded hooks or listeners burned.
Windows timers get more breathing room,
Teardown now is safe and soon! 🎉

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately and specifically summarizes the two main changes: fixing Windows-flaky tests caused by lifecycle teardown race and egg-bin timeouts.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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/windows-flaky-teardown-race

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[cloudflare-workers-and-pages]

Deploying egg with    Cloudflare Pages

Latest commit:	85cbdf6
Status:	✅  Deploy successful!
Preview URL:	https://b2a99b74.egg-cci.pages.dev
Branch Preview URL:	https://fix-windows-flaky-teardown-r.egg-cci.pages.dev

View logs

[gemini-code-assist]

Code Review

This pull request addresses a teardown and in-flight load race condition that causes flaky test failures under Vitest with isolate: false. It introduces an isClosed getter to Lifecycle, skips registering close hooks if the lifecycle is already closed instead of throwing an error, and short-circuits the application loading process early if the lifecycle has closed. The review feedback highlights that when short-circuiting the load process early, other initialized resources like messenger and loggers should also be cleaned up to prevent resource and memory leaks.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[gemini-code-assist]

When short-circuiting load() early due to a teardown race, we must also clean up other resources initialized in the constructor or during the early load phase (such as this.messenger and any lazily created this.#loggers). Otherwise, their process-level event listeners (IPC) and file descriptors will leak, causing state/memory leaks under isolate: false.

    if (this.lifecycle.isClosed) {
      process.removeListener('unhandledRejection', this._unhandledRejectionHandler);
      this.messenger.close();
      if (this.#loggers) {
        for (const logger of this.#loggers.values()) {
          logger.close();
        }
      }
      return;
    }

[killagu]

Good catch — fixed in f0af5ef. The early-return path now closes the messenger (IPC listeners) and any lazily-created loggers (#loggers, file descriptors) in addition to removing the unhandledRejection listener, so nothing leaks across files under isolate: false.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.57%. Comparing base (0333c73) to head (85cbdf6).
⚠️ Report is 1 commits behind head on next.

Additional details and impacted files

@@            Coverage Diff             @@
##             next    #5978      +/-   ##
==========================================
+ Coverage   85.50%   85.57%   +0.06%     
==========================================
  Files         669      669              
  Lines       19825    19849      +24     
  Branches     3917     3923       +6     
==========================================
+ Hits        16952    16986      +34     
+ Misses       2481     2474       -7     
+ Partials      392      389       -3     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[cloudflare-workers-and-pages]

Deploying egg-v3 with    Cloudflare Pages

Latest commit:	85cbdf6
Status:	✅  Deploy successful!
Preview URL:	https://3b0194fb.egg-v3.pages.dev
Branch Preview URL:	https://fix-windows-flaky-teardown-r.egg-v3.pages.dev

View logs

[coderabbitai]

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 `@packages/core/src/lifecycle.ts`:
- Around line 256-266: The guard in the registerBeforeClose() method only checks
if the app has already closed (`#isClosed`), but it does not guard against hooks
being registered while a close operation is in-progress. When close() starts but
has not yet finished, registerBeforeClose() can still accept and store hooks
after the close callback snapshot is taken, causing those hooks to never execute
and leaving teardown cleanup incomplete. Add an additional check for a flag that
tracks whether the close operation is currently in-progress (not just whether it
has finished) alongside the existing `#isClosed` check to prevent hooks from being
registered during the entire close lifecycle.

🪄 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: 4df2e1a5-84c0-4dd4-bb4c-6dd4934deb3a

📥 Commits

Reviewing files that changed from the base of the PR and between 34d8732 and 8b68070.

📒 Files selected for processing (6)

• packages/core/src/lifecycle.ts
• packages/core/test/lifecycle.test.ts
• packages/egg/src/lib/egg.ts
• wiki/concepts/vitest-isolate-false-state-leaks.md
• wiki/index.md
• wiki/log.md

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

packages/core/src/lifecycle.ts (1)

294-319: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Reset #isClosing when close() exits.

Line 298 sets #isClosing = true, but close() never flips it back to false. That makes isClosing remain true after completion (or after a thrown close hook), which breaks the getter’s “currently running” contract.

Suggested fix

  async close(): Promise<void> {
    // Mark closing before the close-callback snapshot is taken, so any hook
    // registered while close() is in flight (e.g. an in-flight load racing
    // teardown) is refused by registerBeforeClose() instead of being stranded.
    this.#isClosing = true;
-   if (this.#metadataOnly || this.#snapshotBuilding) {
-     debug('%s skip beforeClose functions in early-exit lifecycle mode', this.app.type);
-     this.#closeFunctionSet.clear();
-   } else {
-     // close in reverse order: first created, last closed
-     const closeFns = Array.from(this.#closeFunctionSet);
-     debug('%s start trigger %d beforeClose functions', this.app.type, closeFns.length);
-     for (const fn of closeFns.reverse()) {
-       debug('%s trigger beforeClose at %o', this.app.type, fn.fullPath);
-       await utils.callFn(fn);
-       this.#closeFunctionSet.delete(fn);
-     }
-   }
-
-   // Be called after other close callbacks
-   this.app.emit('close');
-   this.removeAllListeners();
-   this.app.removeAllListeners();
-   this.#isClosed = true;
-   debug('%s closed', this.app.type);
+   try {
+     if (this.#metadataOnly || this.#snapshotBuilding) {
+       debug('%s skip beforeClose functions in early-exit lifecycle mode', this.app.type);
+       this.#closeFunctionSet.clear();
+     } else {
+       // close in reverse order: first created, last closed
+       const closeFns = Array.from(this.#closeFunctionSet);
+       debug('%s start trigger %d beforeClose functions', this.app.type, closeFns.length);
+       for (const fn of closeFns.reverse()) {
+         debug('%s trigger beforeClose at %o', this.app.type, fn.fullPath);
+         await utils.callFn(fn);
+         this.#closeFunctionSet.delete(fn);
+       }
+     }
+
+     // Be called after other close callbacks
+     this.app.emit('close');
+     this.removeAllListeners();
+     this.app.removeAllListeners();
+     this.#isClosed = true;
+     debug('%s closed', this.app.type);
+   } finally {
+     this.#isClosing = false;
+   }
  }

🤖 Prompt for 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.

In `@packages/core/src/lifecycle.ts` around lines 294 - 319, The close() method
sets this.#isClosing = true at the start but never resets it to false upon
completion or error, causing the flag to remain true after the method exits and
breaking the getter's contract that it represents whether close is currently
running. Add code to reset this.#isClosing = false when the close() method exits
by either placing it at the end of the method after this.#isClosed = true is
set, or preferably wrapping the method logic in a try-finally block to ensure
the flag is reset even if an error is thrown during close hook execution.

🤖 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 `@packages/egg/src/lib/egg.ts`:
- Around line 267-284: The if (!registered) early-return block cleans up
resources but fails to clear the startup timeout timer that was started in
`#setupTimeoutTimer`(), allowing it to fire after the app is torn down. Add a
private property to track the timeout ID returned by `#setupTimeoutTimer`(),
create a new `#clearStartTimeoutTimer`() method to safely clear this timeout,
modify `#setupTimeoutTimer`() to store the timeout ID and call
`#clearStartTimeoutTimer`() when the app is ready, and finally call
`#clearStartTimeoutTimer`() within the if (!registered) block before returning to
prevent the timer from firing on the torn-down app.

---

Outside diff comments:
In `@packages/core/src/lifecycle.ts`:
- Around line 294-319: The close() method sets this.#isClosing = true at the
start but never resets it to false upon completion or error, causing the flag to
remain true after the method exits and breaking the getter's contract that it
represents whether close is currently running. Add code to reset this.#isClosing
= false when the close() method exits by either placing it at the end of the
method after this.#isClosed = true is set, or preferably wrapping the method
logic in a try-finally block to ensure the flag is reset even if an error is
thrown during close hook execution.

🪄 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: 24b3a0cd-ecf8-4220-ba81-c53cb843fa58

📥 Commits

Reviewing files that changed from the base of the PR and between 8b68070 and f0af5ef.

📒 Files selected for processing (4)

• packages/core/src/lifecycle.ts
• packages/core/test/lifecycle.test.ts
• packages/egg/src/lib/egg.ts
• wiki/concepts/vitest-isolate-false-state-leaks.md

✅ Files skipped from review due to trivial changes (1)

• wiki/concepts/vitest-isolate-false-state-leaks.md

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.