fix: daemon resource thresholds incompatible with macOS (upstream #1078)#5
Open
fix: daemon resource thresholds incompatible with macOS (upstream #1078)#5
Conversation
The daemon canRunWorker() resource check has two threshold defaults that prevent workers from ever executing on macOS: 1. maxCpuLoad: 2.0 — os.loadavg() returns system-wide load (not per-core), so an 8-core Mac under normal workload reports ~8-16, far exceeding 2.0. Changed to 0 (dynamic) with core-aware threshold of cpus().length * 3. 2. minFreeMemoryPercent: 20 — macOS os.freemem() only reports truly unused RAM (not reclaimable cache), typically showing 1-5% on a Mac even with plenty available. Changed to 1%. Also removed hardcoded --quiet flag from background daemon spawn that suppressed all worker event logging to the log file. Fixes ruvnet#1077 (cherry picked from commit f95a4d5)
📝 WalkthroughWalkthroughThe changes modify daemon startup and resource threshold behavior. The background daemon startup no longer suppresses output messages, and resource threshold defaults have been updated with CPU load now using a core-aware dynamic calculation when disabled. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@v3/`@claude-flow/cli/src/services/worker-daemon.ts:
- Around line 248-257: The cpuThreshold calculation can become zero when
os.cpus().length === 0; update the fallback to ensure a minimum core count
(e.g., use Math.max(os.cpus().length, 1)) so the dynamic threshold isn't zero.
Specifically, in the block that computes cpuThreshold (references: cpuThreshold,
this.config.resourceThresholds.maxCpuLoad, os.cpus().length, cpuLoad) replace
the fallback expression with a core-aware expression that applies a floor of 1
core before multiplying (or otherwise enforce a non-zero minimum threshold) so
the subsequent cpuLoad > cpuThreshold check behaves correctly in
containerized/restricted environments.
Comment on lines
+248
to
+257
| // CPU check: use configured threshold, or if 0 (default), compute a | ||
| // dynamic core-aware threshold (3x core count). os.loadavg() returns | ||
| // the system-wide load average, not per-core, so on an 8-core machine | ||
| // a load of 8 means 100% utilization — not overload. | ||
| const cpuThreshold = this.config.resourceThresholds.maxCpuLoad > 0 | ||
| ? this.config.resourceThresholds.maxCpuLoad | ||
| : os.cpus().length * 3; | ||
|
|
||
| if (cpuLoad > cpuThreshold) { | ||
| return { allowed: false, reason: `CPU load too high: ${cpuLoad.toFixed(2)} > ${cpuThreshold}` }; |
There was a problem hiding this comment.
Edge case: os.cpus().length can be 0 in some environments, making the fallback threshold 0.
In certain containerized or restricted environments, os.cpus() may return an empty array. A threshold of 0 would cause cpuLoad > 0 to be almost always true, blocking all workers — ironically the same problem this PR fixes.
Consider adding a floor:
Proposed fix
const cpuThreshold = this.config.resourceThresholds.maxCpuLoad > 0
? this.config.resourceThresholds.maxCpuLoad
- : os.cpus().length * 3;
+ : Math.max(os.cpus().length, 1) * 3;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // CPU check: use configured threshold, or if 0 (default), compute a | |
| // dynamic core-aware threshold (3x core count). os.loadavg() returns | |
| // the system-wide load average, not per-core, so on an 8-core machine | |
| // a load of 8 means 100% utilization — not overload. | |
| const cpuThreshold = this.config.resourceThresholds.maxCpuLoad > 0 | |
| ? this.config.resourceThresholds.maxCpuLoad | |
| : os.cpus().length * 3; | |
| if (cpuLoad > cpuThreshold) { | |
| return { allowed: false, reason: `CPU load too high: ${cpuLoad.toFixed(2)} > ${cpuThreshold}` }; | |
| // CPU check: use configured threshold, or if 0 (default), compute a | |
| // dynamic core-aware threshold (3x core count). os.loadavg() returns | |
| // the system-wide load average, not per-core, so on an 8-core machine | |
| // a load of 8 means 100% utilization — not overload. | |
| const cpuThreshold = this.config.resourceThresholds.maxCpuLoad > 0 | |
| ? this.config.resourceThresholds.maxCpuLoad | |
| : Math.max(os.cpus().length, 1) * 3; | |
| if (cpuLoad > cpuThreshold) { | |
| return { allowed: false, reason: `CPU load too high: ${cpuLoad.toFixed(2)} > ${cpuThreshold}` }; |
🤖 Prompt for AI Agents
In `@v3/`@claude-flow/cli/src/services/worker-daemon.ts around lines 248 - 257,
The cpuThreshold calculation can become zero when os.cpus().length === 0; update
the fallback to ensure a minimum core count (e.g., use
Math.max(os.cpus().length, 1)) so the dynamic threshold isn't zero.
Specifically, in the block that computes cpuThreshold (references: cpuThreshold,
this.config.resourceThresholds.maxCpuLoad, os.cpus().length, cpuLoad) replace
the fallback expression with a core-aware expression that applies a floor of 1
core before multiplying (or otherwise enforce a non-zero minimum threshold) so
the subsequent cpuLoad > cpuThreshold check behaves correctly in
containerized/restricted environments.
🔗 Integration Test Results🔗 Cross-Agent Integration Test ReportSession ID: integration-20260206-223949-5ba1cfdbbb5b39751636dd70b3d07bcbd554c921 Summary
Test Results
Recommendations
Next Steps
Generated by Cross-Agent Integration Test Pipeline |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replica upstream
Questa PR ricrea nel fork la fix upstream:
Contenuto
minFreeMemoryPercentresa compatibile con semanticaos.freemem()su macOS--quiethardcoded nello spawn background per miglior diagnosiMetodo
Cherry-pick del commit upstream
f95a4d55af3172c03219c8212873aca60e893c27(con-x).Summary by CodeRabbit
Release Notes