Uh oh!
There was an error while loading. Please reload this page.
fix(#428): enforce macOS memory floor + clamp recommendations + size colima from RAM - #513
Merged
Merged
Conversation
…e colima from RAM On macOS the memory picture was the Windows WSL2 story with weaker guardrails: - Floor unenforced: the post-Docker runtime recheck (_pf_recheck_runtime_mem, the one point the REAL VM size is known) only WARNed, so a Docker VM below the 5 GB floor proceeded and OOM-crashlooped the client. It now HARD-FAILS a sub-floor VM with the exact fix (Docker Desktop memory / COLIMA_MEMORY) on every OS; a between-floor-and- warn VM still only warns. - Recommendations exceeded physical RAM: "raise to 16 GB" on a ≤16 GB Mac is impossible. New _pf_clamp_mem_gb clamps every SHOWN figure (rec AND warn) to physical − PF_OS_RESERVE_GB (default 2); all _pf_memory / recheck hints use it. - Colima never sized: it was hard-coded --memory 6 (too big for a ≤8 GB Mac, never scaled up). New _macos_vm_mem_gb derives min(half of physical, clamped rec), floored at PF_MIN_MEM_GB, from hw.memsize — the single sizing helper the macOS VM paths share. setup-macos.sh's colima start uses it (COLIMA_MEMORY still overrides). Deferred (noted): writing Docker Desktop's settings.json memoryMiB with consent — the riskiest piece (mutates a user's Docker config) and not in the acceptance criteria; better as its own PR. Tests: _pf_clamp_mem_gb (clamp/headroom/unknown), _macos_vm_mem_gb (8->5 floor, 16->8, 64->16 cap, unknown->default), recheck sub-floor HARD FAIL vs between-floor warn, and a guard that colima memory is derived not hard-coded. Closes#428
…ry-sizing # Conflicts: # scripts/manifest.sha256
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
…bot)
- Clamped hints could undershoot the floor: _pf_clamp_mem_gb capped at physical −
reserve with no lower bound, so a ~6 GB host got "raise to 4 GB" — below the 5 GB
floor AND below the RAM it already has. Floor the clamp at PF_MIN_MEM_GB so a hint
never recommends a sub-floor number (mirrors the PowerShell path's "at least min").
- Colima remedy didn't resize: the hard-fail hint said
`COLIMA_MEMORY=… colima stop && colima start`, but colima doesn't read
COLIMA_MEMORY and the env prefix applies only to `stop`, so `start` kept the old
size. Use the real resize: `colima stop && colima start --memory <N>`.
Also fixes a test bug: `_pf_clamp_mem_gb 16 ''` hit the ${2:-host} default and read
real host RAM (passed on a big-RAM Mac, failed on the smaller CI runner) — the
"can't clamp" case now tests 0 and a non-numeric string.
Tests: never-undershoot-floor on a 6 GB host; colima-resize command guard; corrected
unknown-physical test.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 8dea817. Configure here.
Uh oh!
There was an error while loading. Please reload this page.
…e (Bugbot r2) Catch-22 my sizing introduced: _macos_vm_mem_gb sized colima to EXACTLY the floor (5 on an 8 GB Mac), but a guest's MemTotal runs a few hundred MiB below the configured VM size while the recheck allows only 64 MiB grace — so a fresh install started colima at the derived budget and then hard-failed at cluster-create on the size it had just chosen. And on ≤7 GB hosts the remedy repeated an unachievable size (no way forward). - _macos_vm_mem_gb now sizes ≥ PF_MIN_MEM_GB + 1 (headroom so the guest clears the recheck floor), but never over-commits the host (capped at physical − reserve). An 8 GB Mac -> 6 (clears the recheck); a too-small host gets less and the recheck stops it honestly. - _pf_recheck_runtime_mem: when physical − reserve < floor (host can't ever give the VM the floor), it now says "this Mac has N GB — too little … use a larger machine" instead of a resize remedy that repeats an impossible size. Mirrors the PowerShell host-too-small branch. Tests: 8 GB -> 6; 6 GB host capped to 4 (not over-committed); recheck host-too-small message with no colima-resize line. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
saadqbal
previously approved these changes
Jul 31, 2026
saadqbal
left a comment
Contributor
There was a problem hiding this comment.
Nice PR — floor is enforced at the one point the real VM size is known, recs are clamped, colima is derived, and the bats coverage is thorough. One non-blocking thought inline about the recheck tolerance vs. the documented 5 GB floor.
Uh oh!
There was an error while loading. Please reload this page.
… overhead (reviewer) The recheck tolerated only 64 MiB below the floor, but a guest's MemTotal runs a few hundred MiB under its CONFIGURED size — so a Docker Desktop VM hand-set to exactly the documented 5 GB floor reported ~4.8 GB and hard-failed, making the effective floor a GB higher than we tell people (colima dodged it by sizing floor+1). Add PF_VM_MEM_GRACE_MIB (512) and use it in the recheck's floor comparison so a VM at the documented floor passes (warns), while a genuinely sub-floor VM (e.g. 4 GB) still hard-fails. Test: a ~4.8 GB guest warns, doesn't hard-fail. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
saadqbal
approved these changes
Jul 31, 2026
Uh oh!
There was an error while loading. Please reload this page.
LukasWodka added a commit
that referenced
this pull request
Jul 31, 2026
…lines (#417, bash half) Rebuilt on top of #513, which landed the CLAMP half of this work while this PR sat open. #513 already gives every SHOWN figure `_pf_clamp_mem_gb` (physical − PF_OS_RESERVE_GB, floored at PF_MIN_MEM_GB) and hard-fails a sub-floor Docker VM in the post-Docker recheck. This PR's own `_pf_mem_targets` clamp helper is therefore dropped as redundant — it reuses #513's helper instead. What #513 did NOT fix, and this does: the flip-flop. `_pf_total_mem_kb` preferred the runtime view over the host, so the SAME machine reported "16 GB (host)" on a cold run and "6 GB (Docker VM)" on a warm one, purely on whether Docker happened to be running. Two of its tests asserted that behaviour — one was literally named "the Mac trap". - the `_pf_total_mem_kb` memory selector is deleted. Memory has two distinct truths and each caller now names the one it means: `_pf_host_mem_kb` for a hardware fact, `_pf_runtime_mem_kb` for the budget the pods actually get. (CPU keeps its fallback selector — there is no equivalent advice split.) - `_pf_memory` gates on the MACHINE and prints `Memory: N GB (machine)`. The Linux hard-fail gate, the 64 MiB grace and the MemAvailable check are all unchanged. On a machine below the floor the macOS branch no longer offers a Docker resize remedy — no Docker setting fixes too little physical RAM. - Docker's budget becomes its OWN second line via `_pf_runtime_mem_status`, shown only when a runtime is up AND its budget is meaningfully smaller than the machine (the VM case). Native Linux, where the daemon sees all host RAM, no longer repeats the same number twice. - `_pf_hw_summary_line` reports host RAM — it had the same flip-flop in miniature ("7 GB memory" on a 15 GB WSL2 box). - Linux budget hints drop the Docker Desktop dead end (Bugbot #445): a headless box has no Desktop UI, so the remedy names the VM/cgroup limit instead. - `PF_RUNTIME_MEM_WARNED` latches the budget warning so one run never warns twice about the identical condition. It is tested INSIDE the warn branch, never at the top of `_pf_recheck_runtime_mem`, so it can never gate #513's sub-floor hard-fail; a test pins that (latch set + sub-floor VM still exits non-zero). #513's reviewed recheck copy is left exactly as-is. Tests: preflight.bats 89/89 (11 new — the (machine) label, the two-line output, the Linux no-duplicate case, host-unreadable fallback, the clamped/floored advice, both OS hint shapes, the latch, and the latch-can't-gate-the-hard-fail guard; the two Mac-trap selector tests are replaced by a guard that the selector stays gone). Full suite 659/660 — the one failure is `validate_config: valid config passes`, pre-existing on clean develop and macOS-only (the /var symlink; fixed by #443). bash -n + shellcheck --severity=error + check-style clean; manifest regenerated. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
LukasWodka added a commit
that referenced
this pull request
Jul 31, 2026
…floor (#516) Get-WslConfigMemoryGb writes the WSL2 memory budget into the daily user's .wslconfig during elevated provisioning. It did its own arithmetic -- physical RAM minus a PRIVATE 4 GB reserve, floored at 1 GB -- so it could persist a budget the client cannot run in, and it contradicted the installer's own advice in the same run. Measured before ($env:TB_PESTER="1"; . scripts/install-k8s.ps1): host writes advises (run / train) 8 4 6 / 6 <- below the 5 GB floor: guaranteed OOM crashloop 16 12 8 / 14 32 28 8 / 16 <- over-committed: Windows left 4 GB 6 2 4 / 4 <- doomed budget persisted as if intentional 4 1 2 / 2 8 GB is the important case: a perfectly viable host (8 - 2 GB OS reserve = 6, clear of the 5 GB floor) got memory=4GB while Show-MemoryStatus in the same run said "give Docker up to 6 GB". Rather than re-derive a floor here, the function now DELEGATES to Get-PfMemRecommendation -- the existing single-source helper the advice path uses. The written budget is therefore the advised budget by construction, not by two calculations agreeing: host writes advises (train) 7 memory=5GB 5 (= the floor; 5+2 is the stated practical minimum) 8 memory=6GB 6 (was 4 -> OOM) 16 memory=14GB 14 (was 12) 32 memory=16GB 16 (was 28; Windows keeps 16, and the client cannot 64 memory=16GB 16 use more than the recommended training budget) 6 skipped - 4 skipped - Reserve reconciled: the private ReserveGb=4 is gone and the PARAMETER is gone with it, so no caller can reintroduce the drift. $script:PfOsReserveGb (2) is now the only reserve -- which is what its own comment already claimed ("used to cap recommendations AND to reason about the achievable budget in one place, so the two can't drift", #417 reviewer). Three accessors (Get-PfOsReserveGb / Get-PfMinMemGb / Get-PfRecMemGb) give the printing and writing paths one read path for the same numbers; the reserve accessor fails closed (never 0, which would hand WSL2 the entire host). Host genuinely too small (physical - reserve < floor): the function returns 0 and the caller SKIPS the memory setting instead of persisting a budget known to OOM -- which would also bake the failure into the daily account for every later run and read as intentional to whoever debugs it next. It says so plainly instead ("about 7 GB physical is the practical minimum ... use a larger machine"), the same honest framing the macOS path landed in #513. Still warn-only: Set-DailyUserProvisioning is documented never to fail the install, and Windows memory preflight is warn-only throughout. No headroom fudge is needed on Windows, unlike colima in #513: only a 7 GB host lands exactly on the floor, 7 GB is precisely the practical minimum this code reports, and the Windows recheck warns rather than hard-failing -- so there is no catch-22 where the installer hard-fails on the size it just chose. Scope: Show-MemoryStatus and Get-PfMemRecommendation are deliberately untouched so this does not collide with the open #444 (which rewrites both). Verified forward-compatible -- with #444's floored recommender simulated, every host above returns an identical result, because the too-small gate tests the achievable ceiling rather than the recommender's output. Tests: the two assertions that pinned the old cap/floor are replaced by the new contract -- never below the floor, 8 GB -> 6 not 4, 0 (don't write) when the host can't reach the floor, never over-commits, caps at the training budget, PF_MIN_MEM_GB override honoured, no ReserveGb parameter, and a lock asserting written == advised across seven host sizes. Plus source guards that the caller gates the write on 0 and prints the honest too-small message. Gates: Invoke-Pester scripts/tests/ -> 377 passed / 0 failed; Invoke-ScriptAnalyzer (as CI scopes it) -> 0 errors; check-style.sh clean; check-drift.sh no drift; manifest.sha256 regenerated. Refs #418 Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
LukasWodka added a commit
that referenced
this pull request
Jul 31, 2026
… proceed (#520) * fix(installer): Windows was the one OS that let a sub-floor Docker VM proceed #513 decided a Docker VM below the client's memory floor must STOP the install rather than proceed and OOM-crashloop -- "proceeding is worse than the jarring stop the WARN path used to avoid" -- and implemented that in bash for every OS (_pf_recheck_runtime_mem -> error -> exit 1). The Windows installer never got it. Test-PreflightRuntimeMem just called Show-MemoryStatus, which is warn-only, so Windows printed "it will OOM" and then carried on and OOM-crashlooped. The platform this whole memory story (#417/#418/#428/#444/#516) is about was the one platform still shipping the crash. Enforcement now lives in Test-PreflightRuntimeMem, mirroring bash's split: Show-MemoryStatus stays purely presentational (its documented job -- and the function two PRs just contended over), the recheck grades then enforces. It runs as New-K3dCluster's FIRST statement, so exiting leaves no half-built cluster. The subtlety that makes this safe: Get-PfRuntimeMemGb floors to whole GB, and a guest reports a few hundred MiB BELOW its configured size, so a VM set to exactly the documented 5 GB floor reports ~4.8 and floors to 4. A bare `-lt 5` would have hard-failed a correctly configured machine -- the same trap #513's reviewer caught in bash, but worse here because flooring to whole GB discards up to a GB. So the gate compares MiB against floor - grace: - New Get-PfRuntimeMemMib: the same `docker info` value at MiB precision. - New Get-PfVmMemGraceMib (512, PF_VM_MEM_GRACE_MIB) -- the same constant and the same comparison bash uses, so both installers put the floor in the same place. - The recheck now reads the budget ONCE, in MiB, and derives GB from it, so the number printed and the number enforced on cannot disagree. Flooring (not rounding) is kept deliberately: Step-1 floors too, and #417 exists so the reported figure doesn't flip-flop between the two reads. Remedies stay honest and achievable, matching the copy the advice path already prints: a host that CAN reach the floor gets a resize target clamped to it (min(warn, physical - reserve) -- bash's clamped warn target); a host that cannot (physical - reserve < floor) gets the practical minimum and "run the client on a larger machine", never a resize that repeats an impossible size. A between-floor-and-warn budget still only warns -- it can run, just tightly. TRACEBLOC_SKIP_PREFLIGHT still overrides, and Err names it. Tests: the Describe that asserted warn-only is replaced by the new contract -- sub-floor 4 GB hard-fails; a floor-sized VM reporting 4800 MiB passes; the grace band is bounded on both sides (4607 fails, 4608 passes); daemon-silent is a no-op; between-floor-and-warn only warns; the rec is still capped at host RAM; big-host vs host-too-small remedies; host RAM unreadable still fails; the skip env overrides; and the budget is read exactly once. Plus a parity Describe that reads BOTH sources and asserts bash still hard-fails, Windows hard-fails too and is no longer warn-only, and both name the same grace constant -- so the next divergence fails a test instead of shipping. The old tests mocked Get-PfRuntimeMemGb, which this no longer calls; left as-is they would have passed while testing nothing, so they now mock the MiB reader. Mutation-tested rather than trusted: neutering the gate back to warn-only fails 4 enforcement tests; restoring passes all 11. Gates: Invoke-Pester scripts/tests/ -> 401 passed / 0 failed; Invoke-ScriptAnalyzer as CI scopes it -> 0 errors; check-style.sh clean; check-drift.sh no drift; manifest.sha256 regenerated. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(installer): make the grade and the gate share one boundary (Bugbot) Bugbot caught a self-contradiction I had reasoned about and wrongly accepted: the grade was computed from floor($mib / 1024), so a VM configured at exactly the 5 GB floor (reporting ~4800 MiB) became budget 4, and Show-MemoryStatus printed hard-floor "it will OOM" copy plus a resize hint -- for a machine the grace-aware gate immediately ACCEPTED. We told a correctly configured box it would crash and then carried on. That is precisely the "installer contradicts itself in the same run" pattern #418/#516 existed to remove; bash classifies that band warn-only. I had rejected rounding because Step-1 floors and #417 exists so the reported figure doesn't flip-flop. The fix avoids that trade-off entirely: fold the SAME grace in before flooring. $budget = floor(($mib + $grace) / 1024) - 4800 + 512 -> 5: reports the CONFIGURED size (what the user set and can change), grades in the warn band, gate passes. Consistent. - 4096 + 512 -> 4: still sub-floor, still "it will OOM", gate still fails. Consistent. Because the grade and the gate now pivot on the same constant, their boundaries are the same boundary -- (floor * 1024 - grace) MiB. There is no band that warns "will OOM" yet proceeds, and none that passes while being called sub-floor. The contradiction is impossible by construction, not merely absent at the values I happened to test. Tests: the floor-sized VM is asserted NOT to be told it will OOM and to report its configured 5 GB; plus a boundary-coincidence test sweeping 4096/4607/4608/ 4800/5120 that asserts at EVERY point the copy and the gate agree. Mutation-tested: reverting the grade to floor($mib / 1024) fails both new tests; restoring passes all 13. Gates: Invoke-Pester scripts/tests/ -> 403 passed / 0 failed; Invoke-ScriptAnalyzer as CI scopes it -> 0 errors; check-style.sh clean; check-drift.sh no drift; manifest.sha256 regenerated. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
LukasWodka added a commit
that referenced
this pull request
Jul 31, 2026
develop moved a long way while this sat open (#513 macOS memory floor, #518 storage remedy, #516/#444/#517/#520 on the Windows side, #434 RFC docs). Only scripts/manifest.sha256 conflicted textually — the same single install-k8s.ps1 hash line that has now collided four times today. preflight.sh and preflight.bats auto-merged. Resolution: regenerate the manifest (it is a DERIVED artifact — 18 digests, no secrets; authenticity comes from the release workflow's cosign signature, not from git), so regenerating is the only correct resolution. Taking either side would leave a wrong digest, which the R8 gate then rejects. Verified the auto-merge rather than trusting it. The real hazard here was not the conflict but the clean-looking merge: this branch DELETES _pf_total_mem_kb (the "prefer the runtime" memory selector whose conflation of host RAM and VM budget is the bug it fixes), so any caller that landed on develop meanwhile would have merged into a call to a function that no longer exists — a silent break git reports as success. - _pf_total_mem_kb: undefined and unreferenced after the merge; the only mentions are this branch's own guard test asserting its absence, and a comment. - #518's _pf_storage_type network-FS remedy survived intact. - This branch's _pf_runtime_mem_status is present and still wired into both _pf_memory and _pf_recheck_runtime_mem. Gates: bats scripts/tests/*.bats -> 683 ok / 0 not ok (full TAP plan reported, not a truncated read); shellcheck --severity=error over the CI file set -> rc=0; bash -n clean; Pester -> 403 passed / 0 failed (install-k8s.ps1 arrived via this merge); check-style clean; check-drift no drift; gen-manifest.sh --check current. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
LukasWodka
commented
Aug 2, 2026
Contributor
/fr-pass |
LukasWodka added a commit
that referenced
this pull request
Aug 3, 2026
… clamped (bash half of #417) (#445) * fix(installer): memory truth — machine RAM vs Docker's budget as two lines (#417, bash half) Rebuilt on top of #513, which landed the CLAMP half of this work while this PR sat open. #513 already gives every SHOWN figure `_pf_clamp_mem_gb` (physical − PF_OS_RESERVE_GB, floored at PF_MIN_MEM_GB) and hard-fails a sub-floor Docker VM in the post-Docker recheck. This PR's own `_pf_mem_targets` clamp helper is therefore dropped as redundant — it reuses #513's helper instead. What #513 did NOT fix, and this does: the flip-flop. `_pf_total_mem_kb` preferred the runtime view over the host, so the SAME machine reported "16 GB (host)" on a cold run and "6 GB (Docker VM)" on a warm one, purely on whether Docker happened to be running. Two of its tests asserted that behaviour — one was literally named "the Mac trap". - the `_pf_total_mem_kb` memory selector is deleted. Memory has two distinct truths and each caller now names the one it means: `_pf_host_mem_kb` for a hardware fact, `_pf_runtime_mem_kb` for the budget the pods actually get. (CPU keeps its fallback selector — there is no equivalent advice split.) - `_pf_memory` gates on the MACHINE and prints `Memory: N GB (machine)`. The Linux hard-fail gate, the 64 MiB grace and the MemAvailable check are all unchanged. On a machine below the floor the macOS branch no longer offers a Docker resize remedy — no Docker setting fixes too little physical RAM. - Docker's budget becomes its OWN second line via `_pf_runtime_mem_status`, shown only when a runtime is up AND its budget is meaningfully smaller than the machine (the VM case). Native Linux, where the daemon sees all host RAM, no longer repeats the same number twice. - `_pf_hw_summary_line` reports host RAM — it had the same flip-flop in miniature ("7 GB memory" on a 15 GB WSL2 box). - Linux budget hints drop the Docker Desktop dead end (Bugbot #445): a headless box has no Desktop UI, so the remedy names the VM/cgroup limit instead. - `PF_RUNTIME_MEM_WARNED` latches the budget warning so one run never warns twice about the identical condition. It is tested INSIDE the warn branch, never at the top of `_pf_recheck_runtime_mem`, so it can never gate #513's sub-floor hard-fail; a test pins that (latch set + sub-floor VM still exits non-zero). #513's reviewed recheck copy is left exactly as-is. Tests: preflight.bats 89/89 (11 new — the (machine) label, the two-line output, the Linux no-duplicate case, host-unreadable fallback, the clamped/floored advice, both OS hint shapes, the latch, and the latch-can't-gate-the-hard-fail guard; the two Mac-trap selector tests are replaced by a guard that the selector stays gone). Full suite 659/660 — the one failure is `validate_config: valid config passes`, pre-existing on clean develop and macOS-only (the /var symlink; fixed by #443). bash -n + shellcheck --severity=error + check-style clean; manifest regenerated. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(installer): one threshold + one copy for the Docker budget; no dead-end advice on a tiny host (Bugbot #445 r2) Two findings from Bugbot on c50df6b, both reproduced and both real. 1) The recheck never actually used the "shared" copy, and graded differently. `_pf_runtime_mem_status` was documented as the single copy for preflight AND the post-Docker recheck, but the recheck still printed its own text — so the COLD install path (Docker starts mid-run, the common case) got no colima guidance on macOS and no hint at all on Linux. Worse, the two compared against different thresholds: the helper against the clamped target, the recheck against the raw PF_WARN_MEM_GB. Measured on an 8 GB host (clamped warn = 6) with a 6 GB budget, one run printed both: ✔ Docker's memory budget: 6 GB ⚠ Docker is running with 6 GB — recommended ≥ 6 GB (6 GB to train) Grading now lives only in the helper, and the recheck calls it. The helper takes MiB so it uses the same PF_VM_MEM_GRACE_MIB tolerance as the recheck — rounding to whole GB first misgraded a VM sized to exactly the documented floor (4900 MiB guest) as sub-floor. A `quiet_ok` flag keeps the recheck silent on a healthy budget, so no run prints the same ✔ twice. 2) A machine too small for the floor was still told to resize Docker. On a 4 GB Mac the budget line advised "colima start --memory 5" — more than the machine has, undercutting the honest "use a larger machine" stop the recheck owns. The helper now detects host − PF_OS_RESERVE_GB < PF_MIN_MEM_GB and points at the machine instead. This mirrors the same fix on the PowerShell side (#444), so both installers now agree on the same hardware. The sub-floor HARD-FAIL is untouched and still unconditional: the latch is tested inside the warn branch only, and a test pins that a set latch plus a sub-floor VM still exits non-zero. Tests: preflight.bats 94/94 (5 new — host-too-small gets no resize, a host that CAN reach the floor still does, the preflight-OK'd budget is never re-warned, the recheck is silent when healthy, and the cold path carries the colima guidance). Full suite 664/665 — the one failure is the pre-existing macOS-only `validate_config` case fixed by #443. shellcheck --severity=error and --warning both clean on preflight.sh; bash -n, check-style and check-drift clean; manifest regenerated. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(review): three diverging-copy findings, all closed via one shared predicate Bugbot r3 on #445. All three were the SAME shape as the bug this PR exists to remove — two copies of one judgement disagreeing — surviving on paths the first pass missed. 1. Sub-floor remedy contradicted the hard-fail. On a warm run with a sub-floor budget, _pf_runtime_mem_status hinted "Give Docker <rec>" while _pf_recheck_runtime_mem hard-failed with "raise to <warn>" — the latch suppresses a duplicate warning, deliberately never the hard-fail, so both printed. The remedy now quotes the size that failure quotes; the between-floor-and-warn branch, where no hard-fail follows, still aims at the train figure. 2. Budget GB omitted the VM grace. rt_mib/1024 showed a VM configured at exactly the documented floor as one GB BELOW it — graded correctly by the grace-aware thresholds, displayed as a contradiction. Now (mib + grace)/1024, matching the PowerShell peer. 3. "Enough to run" contradicted "use a larger machine". _pf_memory compared host RAM straight against the Docker floor, ignoring the OS reserve, so a 5-6 GB Mac was graded enough-to-run on one line and told to use a larger machine two lines later. Both now read ONE predicate, _pf_host_too_small_for_floor, which fails safe on unknown input. Native Linux keeps its original wording: the daemon sees host RAM, so the reserve arithmetic does not apply. 696 bats pass (4 new: sub-floor remedy agreement, floor-sized VM display, the machine-line verdict, and the predicate incl. junk input). shellcheck clean; manifest regenerated. * fix(preflight): grace the WARN threshold too, and make the r3 tests enforce Two gaps in 718af66, which otherwise stands as-is — the shared _pf_host_too_small_for_floor predicate is the right shape and is kept. 1. The display became grace-aware but the WARN threshold did not (only the floor one was), so the same self-contradiction reopened one boundary up. Measured on 718af66 with a 32 GB host (warn_eff 8): rt_mib=7680 -> ⚠ budget: 8 GB — recommended ≥ 8 GB rt_mib=8000 -> ⚠ budget: 8 GB — recommended ≥ 8 GB rt_mib=8191 -> ⚠ budget: 8 GB — recommended ≥ 8 GB rt_mib=8192 -> ✔ budget: 8 GB A ~512 MiB band telling the operator to raise a budget to the size it already reports — and Docker Desktop's own defaults land in it. The warn threshold now carries the same grace, so shown == target implies the ✔ branch at BOTH boundaries rather than just the floor. 2. The four r3 tests were only partially enforcing. Under Bats 1.13 a failing bare `[[ ]]` that is not the LAST command in a test body does not fail the test, so `budget: 5 GB`, `!= enough to run`, `Give Docker <warn_eff>` and every line but the last of the predicate test were advisory — they would have passed against broken code. All 16 assertions in those tests now carry `|| return 1`. Verified by mutation, not by inspection: - reverting the warn-threshold grace (i.e. 718af66's shipped state) fails the new boundary test — so this is a real gap, not a hypothetical one; - reverting the display grace fails 2 tests; - neutering _pf_host_too_small_for_floor fails 3, including the r3 predicate test that only became capable of failing once hardened. Baseline and restored are clean in every case. Scope note: only the r3 tests are hardened here. 170 of ~698 tests in this suite share the un-hardened pattern; that sweep needs its own PR because hardening will surface previously-vacuous failures that each need triage (real installer bug vs stale assertion), and burying that in this PR would hide it. Gates: bats scripts/tests/*.bats -> plan 697, ok 697, not ok 0 (complete TAP run); shellcheck --severity=error over the CI file set -> rc=0; check-style clean; check-drift no drift; gen-manifest.sh --check current. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(review): make _pf_memory's thresholds match the grace on its own display Bugbot r5 (High), and correcting my own assumption: the grace on _pf_memory's shown GB came in from develop via the merge, not from my r4 edit. Either way the halves disagreed — the display added PF_VM_MEM_GRACE_MIB while the floor gate kept a 64 MiB tolerance, so a 5 GB VM reporting ~4900 MiB printed "Memory: 5 GB — below the 5 GB the client needs" and, on Linux, hard-failed on it. - floor_mib and warn_mib now use PF_VM_MEM_GRACE_MIB, the same tolerance the display uses and the same one _pf_runtime_mem_status already used for its floor and warn tests. All three now agree on the boundaries. - _pf_host_too_small_for_floor is now fed $(_pf_host_mem_gb) instead of $gb. A shared predicate only prevents divergence if both call sites pass the same input; _pf_memory was passing a grace-adjusted VM-or-host figure while the status path passed raw host GB. MemAvailable is deliberately left ungraced: it is a live measurement, not a configured size, so adjusting it would mask a real shortage. 713 bats pass (2 new: the floor-sized-VM message, and a guard that both call sites feed the predicate the same figure). shellcheck clean; manifest regenerated. * fix(review): render every memory GB through one converter Bugbot r6 found a FOURTH site: _pf_hw_summary_line computed its own memory GB, so the collapsed summary could print a different size from the memory line in the same preflight. Investigating it turned up something worse, and it corrects the record on r4/r5: my r4 fix for _pf_recheck_runtime_mem anchored on a two-line pattern that also existed in _pf_memory, and the replace took the FIRST match — so the fix landed in _pf_memory and the recheck never got it. The grace on _pf_memory's display, which I attributed on the PR to develop via the merge, was actually that misapplied edit. The r5 High finding was a direct consequence. Structural fix rather than a fifth patch: _pf_display_gb_from_mib is now the single definition, used by _pf_memory, _pf_runtime_mem_status, _pf_recheck_runtime_mem and _pf_hw_summary_line. rt_gb goes through it too — it feeds the 'is the VM meaningfully smaller than the machine' comparison, and grading one grace-adjusted side against a raw other side is precisely the mistake these six rounds keep rediscovering. Deliberately still raw: MemAvailable (a live measurement — inflating it would hide a real shortage), disk, and _pf_host_mem_gb (physical RAM needs no compensation, and it is the input _pf_host_too_small_for_floor grades). Every replacement in this commit asserted its anchor matched EXACTLY once and refused otherwise — the guard that would have caught r4's error. 715 bats pass (2 new: a source-level invariant that no site renders its own memory GB, and summary-vs-memory-line agreement). shellcheck clean; manifest regenerated. * fix(review): apply the too-small predicate on every OS in the recheck Bugbot r7 (High). Two divergences in one branch: the reserve arithmetic was inlined instead of calling _pf_host_too_small_for_floor, and the branch was gated OS != Linux while _pf_runtime_mem_status applies it everywhere. On a warm Linux install with a sub-floor cgroup/VM budget, preflight said 'use a larger machine' and this hard-fail then advised raising Docker to a size that machine cannot give. Now calls the shared predicate on every OS, with an OS-appropriate noun so the Mac wording is preserved. Verified both messages agree on a 6 GB Linux host with a 3 GB budget: both say 'use a larger machine'. Same class as r1-r6 on a new axis: not two values disagreeing but two OS GATES disagreeing about when one judgement applies. 717 bats pass (2 new: the Linux path, and an invariant that the reserve arithmetic exists in exactly one place). shellcheck clean; manifest regenerated. --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
3 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.
#428 — enforce the macOS memory floor, clamp recommendations, size colima from RAM
On macOS the memory picture was the Windows WSL2 story with weaker guardrails. Three fixes, one shared sizing helper:
1. The floor is now enforced (was warn-only)
_pf_recheck_runtime_memruns fromcreate_cluster— the one point the real Docker VM size is known. It only WARNed, so a VM below the 5 GB floor sailed through and OOM-crashlooped the client. It now HARD-FAILS a sub-floor VM with the exact fix (Docker Desktop memory /COLIMA_MEMORY); a between-floor-and-warn VM still only warns (the user has already waited for Docker; it can run, just tightly).2. No recommendation exceeds physical RAM
"raise to 16 GB" on a ≤16 GB Mac is impossible. New
_pf_clamp_mem_gb DESIREDclamps every shown figure — both the rec and the warn — tophysical − PF_OS_RESERVE_GB(default 2). All_pf_memory/ recheck hints use it.3. colima is sized from
hw.memsize(was hard-coded 6)--memory 6was too big for a ≤8 GB Mac to spare and never scaled up. New_macos_vm_mem_gbderivesmin(half of physical, clamped recommendation), floored atPF_MIN_MEM_GB— the single sizing helper the macOS VM paths share (preflight.sh, sourced beforesetup-macos.sh).setup-macos.sh'scolima startuses it;COLIMA_MEMORYstill overrides. Examples: 8 GB→5, 16 GB→8, 64 GB→16.Deferred (noted)
Writing Docker Desktop's
settings.jsonmemoryMiBwith consent (scope bullet 3) — the riskiest piece (mutates a user's Docker config) and not in the acceptance criteria. Better as its own PR; the shared sizing helper is already in place for it.Acceptance
_pf_clamp_mem_gbon every shown figure).Tests
_pf_clamp_mem_gb(clamp / headroom / unknown-physical),_macos_vm_mem_gb(8→5 floor, 16→8, 64→16 cap, unknown→default), recheck sub-floor HARD FAIL vs between-floor warn, and a guard that colima memory is derived (not hard-coded 6).shellcheck --severity=error+ check-style + check-drift clean; full preflight.bats green (75);scripts/manifest.sha256regenerated (R8).Closes#428
Note
Medium Risk
Changes install-time exit behavior and Colima VM sizing on macOS; mistakes could block valid installs or size VMs incorrectly, but scope is limited to preflight/setup scripts with extensive bats coverage.
Overview
macOS/Docker memory handling is tightened so undersized VMs stop before the client OOMs, and user-facing numbers match what the machine can actually provide.
Runtime recheck (
_pf_recheck_runtime_mem, called from cluster create oncedocker infois reliable) now hard-fails when the Docker VM is below the ~5 GB floor (withPF_VM_MEM_GRACE_MIBso a VM sized to the documented floor isn’t rejected on guest MemTotal shortfall). VMs between floor and warn still only warn. Hosts that can’t ever allocate a floor-sized VM get a “use a larger machine” message instead of impossible resize advice; Colima fixes usecolima stop && colima start --memory N.Hints and sizing helpers:
_pf_clamp_mem_gbcaps shown warn/rec values to physical RAM minusPF_OS_RESERVE_GB(never below the client floor)._macos_vm_mem_gbpicks Colima/Docker VM budget from half of physical RAM vs clamped recommendation, with headroom above the floor.setup-macos.shstarts Colima with that derived memory instead of a fixed 6 GB (COLIMA_MEMORYstill overrides).Tests cover clamp/VM sizing, recheck hard-fail vs warn vs grace, host-too-small, and Colima wiring;
scripts/manifest.sha256updated.Reviewed by Cursor Bugbot for commit a125545. Bugbot is set up for automated code reviews on this repo. Configure here.