feat(doctor): add WSL environment diagnostics check [AI-assisted]#58853
feat(doctor): add WSL environment diagnostics check [AI-assisted]#58853alkor2000 wants to merge 5 commits intoopenclaw:mainfrom
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5fc1f5033a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Greptile SummaryThis PR adds a new One P1 logic bug was found:
P2 note:
Confidence Score: 4/5Safe to merge after fixing the os.totalmem() double-halving bug — the rest of the contribution is well-structured and correctly guarded. One P1 logic bug (the missing-.wslconfig memory-threshold check uses a doubly-halved value derived from os.totalmem(), producing both a spurious warning and an incorrect user-facing message) should be resolved before merging. Everything else — the INI parser, path resolution strategies, kernel-version reader, systemd summary, and Doctor registration — looks correct and follows existing patterns. src/commands/doctor-wsl.ts — the buildWSLDiagnosticNotes missing-.wslconfig branch (lines 321–332) and the hostTotalMemoryBytes field naming in WSLDiagnostics. Prompt To Fix All With AIThis is a comment left during a code review.
Path: src/commands/doctor-wsl.ts
Line: 322-329
Comment:
**`os.totalmem()` reports WSL allocation, not Windows host total — double-halving the value**
Inside WSL, `os.totalmem()` returns the memory allocated *to the WSL instance* (WSL2's default is ~50% of host RAM), not the actual Windows host total. `buildWSLDiagnosticNotes` then divides that value by 2 again to estimate "WSL's default allocation", effectively dividing the host memory by 4.
Concrete example on an 8 GB Windows host (no `.wslconfig`):
- `os.totalmem()` → ~4 GB (WSL default allocation)
- `hostMemGB` → 4
- `wslDefaultMemGB` → `⌊4/2⌋ = 2`
- Warning fires: *"WSL defaults to ~2 GB memory (half of 4 GB host)"*
Both numbers in the message are wrong: WSL already has 4 GB (which meets the stated ≥4 GB threshold), and the Windows host has 8 GB, not 4 GB. The diagnostic fires spuriously (WSL is fine) while also showing misleading figures.
The simplest correct fix is to compare `os.totalmem()` directly against the 4 GB threshold — it already represents the WSL allocation:
```suggestion
const hostMemGB = Math.round(diag.hostTotalMemoryBytes / (1024 * 1024 * 1024));
if (hostMemGB > 0 && hostMemGB < 4) {
notes.push(
`No .wslconfig found. WSL is currently limited to ~${hostMemGB}GB memory.`,
);
notes.push("Tip: create %USERPROFILE%\\.wslconfig to set explicit resource limits.");
}
```
You may also want to rename `hostTotalMemoryBytes` → `wslTotalMemoryBytes` in `WSLDiagnostics` and the `collectWSLDiagnostics` return value, since the value is WSL-scoped and the current name is actively misleading.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: src/commands/doctor-wsl.ts
Line: 25-26
Comment:
**Duplicate import from the same module**
`isWSL` and `isWSL2Sync` are imported in two separate statements from the same source. Combine them into one:
```suggestion
import { isWSL, isWSL2Sync } from "../infra/wsl.js";
```
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "feat(doctor): add WSL environment diagno..." | Re-trigger Greptile |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d953f2c1c9
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
This comment was marked as spam.
This comment was marked as spam.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 20cf0ed12f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (diag.wslconfig) { | ||
| const memoryMB = parseMemoryToMB(diag.wslconfig.memory); |
There was a problem hiding this comment.
Treat partial wsl2 settings as unconfigured limits
If .wslconfig exists but [wsl2] does not set memory or processors (for example only swap), this path is treated as "configured" and skips the fallback wslVisibleMemoryBytes check. In that case low-memory WSL2 environments (<4GB visible RAM) can pass with no warning even though no memory limit was explicitly configured. Consider treating resource-empty [wsl2] settings as unconfigured (or running the fallback check when memory is missing).
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Acknowledged — this is a valid edge case. In practice, .wslconfig files that have a [wsl2] section almost always include a memory setting. I'd rather keep the logic simple for now and address partial-config handling in a follow-up if maintainers prefer. Happy to add the refinement in this PR if requested.
20cf0ed to
6c41dc5
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6c41dc551a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0abe7afd03
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const wslMemGB = Math.round(diag.wslVisibleMemoryBytes / (1024 * 1024 * 1024)); | ||
| if (wslMemGB > 0 && wslMemGB < 4) { |
There was a problem hiding this comment.
Compare WSL memory threshold before rounding
The missing-.wslconfig branch rounds wslVisibleMemoryBytes to whole GB before evaluating < 4, which suppresses warnings for low-memory environments between 3.5GB and 4GB (rounded to 4) and even for very small limits under 0.5GB (rounded to 0, then filtered by > 0). This can hide the main remediation note on constrained WSL2 setups; compare bytes (or MB) directly against the 4GB threshold and only round for display text.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Acknowledged — the rounding does create a narrow blind spot between 3.5–4GB. In practice, WSL's default allocation (50% of host RAM) makes this range uncommon, and 3.5GB+ is borderline acceptable for OpenClaw. The > 0 guard handles the degenerate case where totalmem() returns 0 or fails.
Happy to switch to byte-level comparison if maintainers prefer, but keeping it simple for now as the diagnostic is informational rather than blocking.
|
All review feedback addressed across 5 commits (force-pushed to clean history):
Tests: 30/30 passed (27 original + 2 byte-parsing + 1 explicit byte suffix). CI: The single failing check ( |
0abe7af to
9c17f57
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9c17f57e66
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
This pull request has been automatically marked as stale due to inactivity. |
|
Still active — will rebase to resolve the doctor-health-contributions.ts conflict shortly. |
Add a new Doctor health contribution that detects WSL/WSL2 and reports environment context and resource configuration issues: - Environment summary: WSL version, kernel, systemd status, memory limit, and processor count displayed in a single line - Resource checks: warns when .wslconfig memory is below 4GB or processor count is below 2 cores - Reliable path resolution: uses wslvar/wslpath to locate the Windows-side .wslconfig, with heuristic fallback The check activates only on Linux+WSL and silently skips on all other platforms. systemd diagnostics are intentionally left to the existing gateway daemon flow to avoid duplicate messaging. Includes 27 unit tests covering the INI parser, memory string parser, diagnostic note builder, and summary builder.
- Fix P1: remove double-halving of os.totalmem() — inside WSL this already reflects the VM allocation, not Windows host total. Compare directly against 4GB threshold instead of halving again. - Fix P2: merge duplicate import from ../infra/wsl.js into one line. - Rename hostTotalMemoryBytes → wslVisibleMemoryBytes to accurately reflect that os.totalmem() in WSL returns VM-scoped memory. - Update tests to match renamed field and corrected logic.
…ection as unconfigured - Gate .wslconfig resource diagnostics behind isWSL2 — WSL1 does not use .wslconfig for resource allocation, so the advice is misleading. - Return null from readWSLConfigResources() when .wslconfig has no [wsl2] section, so the missing-config fallback path still fires. - Add WSL1 test cases to verify resource checks are skipped.
.wslconfig treats bare numbers (no unit suffix) as bytes, not megabytes. A value like memory=2147483648 (2 GiB) was being parsed as ~2 TiB, causing the low-memory warning to never fire. - Change parseMemoryToMB default unit from 'mb' to 'b' - Add bytes-to-MB conversion (num / 1024 / 1024) - Accept explicit 'b' suffix in the regex - Update tests: bare numbers now parse as bytes - Add test for explicit 'b' suffix Addresses Codex P2 review feedback.
9c17f57 to
f46daf3
Compare
|
Codex review: needs changes before merge. Summary Reproducibility: not applicable. as an end-user bug report. The remaining PR defects have high-confidence unit-level repros by constructing Next step before merge Security Review findings
Review detailsBest possible solution: Land the Doctor WSL diagnostic after a focused PR update fixes the remaining helper edge cases, adds regression tests and a changelog entry, then validates the rebased branch. Do we have a high-confidence way to reproduce the issue? Not applicable as an end-user bug report. The remaining PR defects have high-confidence unit-level repros by constructing Is this the best way to solve the issue? No, not as-is. The feature belongs in Doctor and reuses the right WSL helpers, but the patch should avoid misleading or missing diagnostics before it ships. Full review comments:
Overall correctness: patch is incorrect Acceptance criteria:
What I checked:
Likely related people:
Remaining risk / open question:
Codex review notes: model gpt-5.5, reasoning high; reviewed against 9bedcff904dd. |
Summary
Add a Doctor health contribution that proactively detects WSL/WSL2
environments and surfaces environment context + resource configuration
issues that are not covered by any existing check.
Problem
WSL2 is the recommended way to run OpenClaw on Windows, but
openclaw doctorhas no dedicated WSL diagnostics. WSL-related hints arescattered across the gateway daemon flow (systemd-unavailable only) and
onboard (win32 platform only). Users hitting WSL-specific issues —
low memory limits, insufficient processor allocation, or missing
.wslconfig — get no guidance from Doctor.
Changes
New file:
src/commands/doctor-wsl.tssrc/infra/wsl.tshelperssystemd status, memory limit, processor count
.wslconfigfrom the Windows side usingwslvar/wslpathfor reliable path resolution (with heuristic fallback)
.wslconfigwhen host memory is limitedto avoid duplicate messaging
New file:
src/commands/doctor-wsl.test.tsdiagnostic note builder, and summary builder
Modified:
src/flows/doctor-health-contributions.tsdoctor:wsl-environmentcontribution (inserted beforedoctor:systemd-linger)Testing
pnpm check— 0 errors, 0 warningspnpm vitest run src/commands/doctor-wsl.test.ts— 27/27 passedpnpm build— clean buildsystemd enabled, .wslconfig with 8GB/4 processors)
AI Disclosure
/etc/wsl.conf, .wslconfig, and /proc/version to collect WSL
environment info, then emits Doctor notes via the standard
note() API when resource issues are detected
contribution system, code generation, test generation, and
iterative lint/test fixes
Notes
(createDoctorHealthContribution + note())