feat(doctor): add disk space health check for state directory#59196
Conversation
Greptile SummaryThis PR adds a new Key changes:
Findings:
Confidence Score: 5/5
Prompt To Fix All With AIThis is a comment left during a code review.
Path: src/commands/doctor-disk-space.ts
Line: 48-51
Comment:
**Misleading comment: `bsize` is not the "fundamental block size"**
The inline comment says _"Multiply by `bsize` (fundamental block size)"_, but in the libuv/Node.js `StatsFs` API `bsize` is the **optimal transfer block size** (`f_bsize`). The "fundamental block size" term refers to `frsize` (`f_frsize`) from the POSIX `statvfs` struct.
The math itself is correct — in the `statfs`-based API that Node.js exposes, `bavail` blocks are indeed counted in `f_bsize` units — but the comment is the wrong way round and will mislead anyone who checks against POSIX `statvfs` docs.
```suggestion
// `bavail` is the number of free blocks available to unprivileged users.
// Multiply by `bsize` (optimal transfer / filesystem block size in Node's
// StatsFs, equal to `f_bsize` from the underlying statfs syscall) to get
// available bytes.
return Number(stats.bavail) * Number(stats.bsize);
```
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-disk-space.ts
Line: 92-98
Comment:
**Unused `cfg` parameter**
`cfg: OpenClawConfig` is declared as the first parameter but is never read inside the function body. The state directory is resolved entirely from `env` (via `resolveStateDir`), which is also the pattern used by `resolveStateDir` itself — there is no config-level state-dir override.
If the parameter is kept for API symmetry with other Doctor contributions, a short comment clarifying that intent would prevent future readers from wondering whether it was accidentally forgotten. Alternatively, it can be dropped and the call-site in `doctor-health-contributions.ts` updated accordingly.
```suggestion
export function noteDiskSpace(
_cfg: OpenClawConfig, // reserved for API consistency with other Doctor contributions
deps?: {
env?: NodeJS.ProcessEnv;
statfsSync?: (p: string) => fs.StatsFs;
},
): void {
```
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "feat(doctor): add disk space health chec..." | Re-trigger Greptile |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0f0ea00c7a
ℹ️ 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: 971d9e8ad1
ℹ️ 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".
971d9e8 to
41257c5
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a07e4cb913
ℹ️ 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".
bb9aeac to
29459a3
Compare
|
Codex review: needs maintainer review before merge. Reviewed May 31, 2026, 2:34 PM ET / 18:34 UTC. Summary PR surface: Source +125, Tests +169. Total +294 across 4 files. Reproducibility: not applicable. as a bug reproduction: this is a feature PR adding a new Doctor diagnostic. The PR body and latest check-runs provide after-fix terminal proof for healthy and injected low-space readings, and source inspection confirms current main lacks this contribution. Review metrics: none identified. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Next step before merge
Security Review detailsBest possible solution: Land the PR after normal maintainer approval, keeping disk probing in src/infra/disk-space.ts and the Doctor-only thresholds/output in src/commands/doctor-disk-space.ts. Do we have a high-confidence way to reproduce the issue? Not applicable as a bug reproduction: this is a feature PR adding a new Doctor diagnostic. The PR body and latest check-runs provide after-fix terminal proof for healthy and injected low-space readings, and source inspection confirms current main lacks this contribution. Is this the best way to solve the issue? Yes. The current PR shape is the narrow maintainable solution because it reuses the existing shared disk-space helper, keeps Doctor-specific thresholds local, registers the contribution before state integrity, and adds conversion-plan plus unit coverage. AGENTS.md: found and applied where relevant. Codex review notes: model gpt-5.5, reasoning high; reviewed against 7e0d275f7add. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +125, Tests +169. Total +294 across 4 files. View PR surface stats
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
29459a3 to
a723194
Compare
a723194 to
3382444
Compare
|
Real-world evidence: disk-full failures surface as opaque symptoms in GitHub Issues — config corruption (#issue examples), missing sessions, silent write failures. Users cannot diagnose the root cause through Doctor because no check exists. The upstream fix in v2026.4.5 ("surface explicit disk-full message when ENOSPC") confirms this is a recognized problem — this PR adds the proactive diagnostic counterpart. |
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
ClawSweeper PR egg ✨ Hatched: 🥚 common Mossy Branchling Hatch commandComment Hatchability rules:
Rarity: 🥚 common. What is this egg doing here?
|
3382444 to
15b908c
Compare
|
@clawsweeper re-review Addressed the review feedback:
|
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
Heads up: this PR needs to be updated against current |
110b1fe to
b3d760c
Compare
|
Thanks @RomneyDa — rebased onto current main so the Dependency Guard check can run. One adjustment was needed: upstream extracted the terminal |
Add a new Doctor health contribution that checks free disk space on the
partition containing the state directory. Warns when available space drops
below 500 MB and reports CRITICAL when below 100 MB.
This catches a common operational failure mode where OpenClaw silently
fails to write config, sessions, or logs because the disk is full.
- New file: src/commands/doctor-disk-space.ts
- formatBytes(): human-readable byte formatting
- getAvailableBytes(): statfsSync wrapper with error handling
- buildDiskSpaceWarnings(): pure function for threshold logic
- noteDiskSpace(): Doctor contribution entry point
- New file: src/commands/doctor-disk-space.test.ts (20 tests)
- Modified: src/flows/doctor-health-contributions.ts
- Register doctor:disk-space before state-integrity so disk-full
root cause appears before downstream symptoms
- Fix misleading bsize comment: 'fundamental block size' -> 'optimal transfer / filesystem block size' (Greptile P2) - Rename cfg to _cfg with comment for API consistency intent (Greptile P2) - Use Math.floor instead of toFixed(0) for MB/KB formatting to avoid rounding 99.6 MB up to '100 MB' in warning text (Codex P2)
When the state directory does not exist yet (first run, manual cleanup), statfsSync fails and the disk space check silently exits. This suppresses the root-cause warning in low-disk situations. Walk up from the state directory path to find the nearest existing ancestor and probe that instead. This ensures disk pressure is reported even before the state directory is created. - Add findExistingAncestor() with safety bound (max 64 levels) - Update getAvailableBytes() to use ancestor fallback - Add 4 new tests for findExistingAncestor + ancestor probe path (24 tests total) Addresses Codex P2 review feedback.
Replace hardcoded POSIX paths ('/tmp', '/') with os.tmpdir() and
path.parse().root so tests pass on Windows where path.resolve
produces drive-rooted paths (e.g. C:\tmp).
Addresses Codex P1 review feedback.
Address ClawSweeper P1: delete the duplicate statfs/ancestor probing in doctor-disk-space.ts and delegate to src/infra/disk-space.ts (tryReadDiskSpace), keeping a single disk-space implementation shared with install/update diagnostics. The two-tier warning/critical thresholds and Doctor-facing byte formatting remain specific to this health check. Update tests to inject readDiskSpace and drop the now-removed helpers.
noteDiskSpace is synchronous (returns void); awaiting it tripped the no-await-sync lint rule. Drop the await on the call (the dynamic import remains awaited). Also addresses the earlier review note to call the synchronous disk-space note directly.
The doctor-health-conversion-plan consistency test requires every registered health contribution to have a matching conversion rule. Add the doctor:disk-space rule (terminal-side-effect: emits note() warnings, no repair) so the planned and registered contribution id lists stay in sync.
…action Upstream extracted the terminal note module from src/terminal/note.js to packages/terminal-core/src/note.js; update the disk-space contribution and its tests to the new path.
b3d760c to
06b63e4
Compare
steipete
left a comment
There was a problem hiding this comment.
Land-ready review for PR #59196.
What landed in this PR:
- Adds
doctor:disk-space, warning below 500 MB free and reporting critical below 100 MB on the partition containing the active OpenClaw state directory. - Reuses
src/infra/disk-space.tsfor disk probing and nearest-existing-ancestor handling; Doctor owns only thresholds and terminal output. - Registers the contribution before state integrity and records it in the Doctor health conversion plan.
Proof checked:
pnpm test src/commands/doctor-disk-space.test.ts src/flows/doctor-health-conversion-plan.test.tspassed locally: 2 Vitest shards, 23 tests.git diff --check origin/main...HEADpassed locally on the PR head.git merge-tree --write-tree origin/main refs/remotes/pr/59196passed after refreshingmainto5df0ed3b9f5.- GitHub reports
mergeStateStatus: CLEAN,mergeable: MERGEABLEfor head06b63e405c91320466050a45eea2b56558298e15. - CI run
26720861380is green for the relevant required checks; latest Real behavior proof run26720996848is green.
Known gaps:
- I did not run a full local suite; the change is scoped to Doctor command wiring, the disk-space contribution, and its focused tests.
Add a Doctor health contribution that checks free space on the partition containing the active OpenClaw state directory. Doctor now warns below 500 MB and reports critical below 100 MB so disk pressure is visible before config writes, session transcripts, or log rotation start failing. The contribution reuses the shared `src/infra/disk-space.ts` probe, runs before state integrity, and is registered in the Doctor health conversion plan with focused coverage for thresholds, formatting, and note behavior. PR: openclaw#59196 Proof: `pnpm test src/commands/doctor-disk-space.test.ts src/flows/doctor-health-conversion-plan.test.ts`; `git diff --check origin/main...HEAD`; `git merge-tree --write-tree origin/main refs/remotes/pr/59196`; GitHub CI run `26720861380`; Real behavior proof run `26720996848`. Co-authored-by: alkor2000 <[email protected]>
Add a Doctor health contribution that checks free space on the partition containing the active OpenClaw state directory. Doctor now warns below 500 MB and reports critical below 100 MB so disk pressure is visible before config writes, session transcripts, or log rotation start failing. The contribution reuses the shared `src/infra/disk-space.ts` probe, runs before state integrity, and is registered in the Doctor health conversion plan with focused coverage for thresholds, formatting, and note behavior. PR: openclaw#59196 Proof: `pnpm test src/commands/doctor-disk-space.test.ts src/flows/doctor-health-conversion-plan.test.ts`; `git diff --check origin/main...HEAD`; `git merge-tree --write-tree origin/main refs/remotes/pr/59196`; GitHub CI run `26720861380`; Real behavior proof run `26720996848`. Co-authored-by: alkor2000 <[email protected]>
Add a Doctor health contribution that checks free space on the partition containing the active OpenClaw state directory. Doctor now warns below 500 MB and reports critical below 100 MB so disk pressure is visible before config writes, session transcripts, or log rotation start failing. The contribution reuses the shared `src/infra/disk-space.ts` probe, runs before state integrity, and is registered in the Doctor health conversion plan with focused coverage for thresholds, formatting, and note behavior. PR: openclaw#59196 Proof: `pnpm test src/commands/doctor-disk-space.test.ts src/flows/doctor-health-conversion-plan.test.ts`; `git diff --check origin/main...HEAD`; `git merge-tree --write-tree origin/main refs/remotes/pr/59196`; GitHub CI run `26720861380`; Real behavior proof run `26720996848`. Co-authored-by: alkor2000 <[email protected]>
Summary
Add a new Doctor health contribution that checks free disk space on the partition containing the state directory (~/.openclaw). Warns when available space drops below 500 MB and reports CRITICAL when below 100 MB.
Problem
When the disk hosting the state directory fills up, OpenClaw silently fails to write config files, session transcripts, and logs. Users see symptoms (missing sessions, corrupted config) but have no way to diagnose the root cause through Doctor.
The existing doctor:state-integrity contribution checks directory existence, permissions, cloud sync, and SD/eMMC storage — but not free space.
The upstream fix in v2026.4.5 (
surface explicit disk-full message when ENOSPC) confirms this is a recognized problem — this PR adds the proactive diagnostic counterpart.Real behavior proof
Behavior or issue addressed: When the disk hosting the state directory fills up, OpenClaw silently fails to write config, session transcripts, and logs. Doctor had no disk-space check, so users only saw downstream symptoms. This adds a
doctor:disk-spacehealth contribution with two-tier warning (500 MB) / critical (100 MB) thresholds.Real environment tested: Linux (Ubuntu 24.04, WSL2), Node.js 24.14.0, OpenClaw built from this PR head. Exercised the patched
noteDiskSpace()and the sharedtryReadDiskSpace()probe against the real~/.openclawpartition and at simulated low-space thresholds.Exact steps or command run after this patch: invoked the patched functions via tsx against the real state-dir partition and injected low-space readings:
tryReadDiskSpace()helper (now reused instead of a duplicate statfs implementation), plus actualnoteDiskSpace()terminal output at low-space thresholds:Observed result after fix: On a healthy disk (935 GB free) no warning is emitted. At 400 MB the WARNING tier fires; at 80 MB the CRITICAL tier fires with data-loss guidance. Disk probing now delegates to the shared
src/infra/disk-space.tshelper (addressing the duplicate-implementation review note); the two-tier thresholds and formatting remain specific to this Doctor check. 20 unit tests pass.What was not tested: A literally full partition (real ENOSPC) was not reproduced; low-space readings were injected via the shared probe's return value. Non-Linux platforms rely on the shared helper's platform handling.
Solution
New doctor:disk-space contribution registered before doctor:state-integrity so the root cause (disk full) appears before downstream symptoms (write failures).
New file: src/commands/doctor-disk-space.ts
formatBytes()— human-readable byte formatting (B/KB/MB/GB)getAvailableBytes()— statfsSync wrapper using bavail * bsizebuildDiskSpaceWarnings()— pure function for threshold logicnoteDiskSpace()— Doctor contribution entry pointNew file: src/commands/doctor-disk-space.test.ts
Modified: src/flows/doctor-health-contributions.ts
Design decisions
Verification