fix(health-check): resolve Jest worker process leak [TD-6]#35
fix(health-check): resolve Jest worker process leak [TD-6]#35Pedrovaleriolopez merged 2 commits intomainfrom
Conversation
Root Cause: setTimeout in engine.js runSingleCheck() was never cleared after Promise.race resolved, leaving 7 open handles per test run. Changes: - engine.js: Store timeoutId and call clearTimeout() on success/error - dev-context-loader.js: Handle null coreConfig gracefully Before: 7 open handles, Jest exit code 1, "worker process failed to exit" After: 0 open handles, Jest exits cleanly Closes #34 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Note
|
| Cohort / File(s) | Summary |
|---|---|
Core timeout cleanup .aios-core/core/health-check/engine.js |
Adds per-check timeoutId storage, ensures clearTimeout(timeoutId) on both success and error paths (including in runSingleCheck()), and adjusts timeout promise handling to avoid lingering timers that can leak Jest workers. |
Development tooling updates .aios-core/development/scripts/dev-context-loader.js |
Adds safe access for coreConfig.devLoadAlwaysFiles (fallback to []) and standardizes arrow-function parameter parentheses and minor formatting; no behavioral changes. |
Install manifest .aios-core/install-manifest.yaml |
Updates generated_at timestamp and sha256/size entries for the modified files. |
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~20 minutes
Suggested labels
core, ci/cd, bug-fix, tests
Pre-merge checks
✅ 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 describes the main fix: resolving a Jest worker process leak in the health-check engine by clearing timeouts, directly addressing the PR's primary objective. |
| Linked Issues check | ✅ Passed | The PR directly addresses the acceptance criteria in issue #34: identifies the timeout leak source, implements proper cleanup with clearTimeout calls, and the validation confirms 0 open handles post-fix. |
| Out of Scope Changes check | ✅ Passed | The dev-context-loader.js changes (null-safety and formatting) are minor and complementary to the primary fix; manifest updates are expected regeneration artifacts. All changes align with addressing the Jest worker leak issue. |
| Docstring Coverage | ✅ Passed | No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. |
📜 Recent review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.aios-core/install-manifest.yaml
🔇 Additional comments (1)
.aios-core/install-manifest.yaml (1)
11-11: LGTM! Manifest correctly regenerated.The manifest has been properly regenerated to reflect the timeout cleanup fix in
engine.jsand the null-safety improvements indev-context-loader.js. The updated hashes and sizes for these two files are consistent with the TD-6 changes described in the PR objectives.Also applies to: 364-366, 664-666
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.
Comment @coderabbitai help to get the list of available commands and usage tips.
📊 Coverage ReportCoverage report not available
Generated by PR Automation (Story 6.1) |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.aios-core/core/health-check/engine.js (1)
1-1: Pipeline failure: Regenerate the manifest.The CI pipeline is failing because the manifest file is outdated. Please run
npm run generate:manifestto update the manifest with the new file hashes..aios-core/development/scripts/dev-context-loader.js (1)
1-1: Pipeline failure: Regenerate the manifest.The CI pipeline is failing because the manifest file is outdated. Please run
npm run generate:manifestto update the manifest with the new file hashes.
🧹 Nitpick comments (1)
.aios-core/development/scripts/dev-context-loader.js (1)
64-64: Formatting consistency: Arrow function parentheses.The added parentheses around single arrow function parameters improve stylistic consistency across the file. This aligns with common linting rules that enforce uniform parameter syntax.
Also applies to: 167-167, 176-176, 261-261, 290-290
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.aios-core/core/health-check/engine.js.aios-core/development/scripts/dev-context-loader.js
🧰 Additional context used
🪛 GitHub Actions: CI
.aios-core/development/scripts/dev-context-loader.js
[error] 1-1: Manifest is OUTDATED. Modified file hash mismatch detected. To fix, run 'npm run generate:manifest'.
.aios-core/core/health-check/engine.js
[error] 1-1: Manifest is OUTDATED. Modified file hash mismatch detected. To fix, run 'npm run generate:manifest'.
🔇 Additional comments (5)
.aios-core/core/health-check/engine.js (4)
249-249: LGTM: Timeout ID initialization.The timeout ID is correctly initialized before the try block, allowing it to be accessed in both success and error paths for cleanup.
252-259: LGTM: Timeout promise correctly captures timeoutId.The timeout promise properly stores the timeout ID, enabling cleanup in both resolution paths. The timeout duration calculation correctly respects both the check-specific timeout and the overall timeout.
264-268: LGTM: Success path cleanup correctly implemented.The timeout is properly cleared after the check completes successfully. The guard check is defensive programming, and clearing the timeout ensures Jest doesn't detect an open handle.
293-297: LGTM: Error path cleanup ensures no leaks.The timeout is properly cleared in the error handler, ensuring cleanup occurs even when checks fail or time out. This completes the fix for the Jest worker leak, as the timeout is now cleared in all possible code paths.
.aios-core/development/scripts/dev-context-loader.js (1)
40-42: LGTM: Null-safety improvement for coreConfig.The defensive null check is appropriate. While
loadCoreConfig()returns an empty object on error, the YAML parser could potentially returnnullorundefinedfor empty/invalid YAML content, making this guard necessary.
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
* fix(health-check): resolve Jest worker process leak [TD-6] Root Cause: setTimeout in engine.js runSingleCheck() was never cleared after Promise.race resolved, leaving 7 open handles per test run. Changes: - engine.js: Store timeoutId and call clearTimeout() on success/error - dev-context-loader.js: Handle null coreConfig gracefully Before: 7 open handles, Jest exit code 1, "worker process failed to exit" After: 0 open handles, Jest exits cleanly Closes SynkraAI#34 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * chore: regenerate install manifest after engine.js fix 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> --------- Co-authored-by: Pedro Valerio <pedro@allfluence.ai> Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
Summary
setTimeoutinengine.js:runSingleCheck()was never cleared afterPromise.raceresolvedtimeoutIdand callclearTimeout()on both success and error pathsChanges
.aios-core/core/health-check/engine.jsclearTimeout(timeoutId)after Promise.race resolves.aios-core/development/scripts/dev-context-loader.jscoreConfiggracefullyValidation
Tests: 17 health-check tests passing, 0 open handles detected with
--detectOpenHandlesTest plan
npm test -- tests/health-check/health-check.test.js --detectOpenHandlesnpm testto confirm all tests passRelated
🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.