Skip to content

fix(installer): make the bats suite green on stock macOS (bash 3.2 + tmpdir symlink + host-CLI leak) - #443

Merged
LukasWodka merged 2 commits into
developfrom
fix/bats-macos-portability
Jul 31, 2026
Merged

fix(installer): make the bats suite green on stock macOS (bash 3.2 + tmpdir symlink + host-CLI leak)#443
LukasWodka merged 2 commits into
developfrom
fix/bats-macos-portability

Conversation

@LukasWodka

@LukasWodkaLukasWodka commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Summary

Three bats tests failed when the suite runs locally on macOS but passed in ubuntu CI (verified on a clean origin/develop checkout) — noise that masks real regressions for anyone running bats scripts/tests/*.bats before pushing. All three are local-environment artifacts; this fixes each at its actual root cause and silences the one bats warning the suite emitted. Suite is now 468/468 green on stock macOS (bash 3.2, bats 1.13) with zero warnings, and unchanged on Linux.

Root causes & fixes

1. _extract_yaml_value: single-quoted with '' escape (install-client-helm.bats) — bash 3.2 bug in the lib code.
In ${var//pat/rep}, macOS's bash 3.2 keeps the backslash when the replacement is an escaped quote: the '' de-escape turned 'a''b' into a\'b (bash 4+/5 gives a'b). Fixed by building pattern/replacement from a $sq variable — identical semantics on every bash. The mirror-image escape site (' -> '' for TB_CLIENT_PASSWORD_ESCAPED) had the same latent bug and gets the same two-line fix. scripts/manifest.sha256 regenerated since this lib is on the R8 integrity surface (gen-manifest.sh --check passes).

2. validate_config: valid config passes (common.bats) — tmpdir symlink shape.
macOS puts BATS_TEST_TMPDIR under the /var -> /private/var symlink. validate_config resolves the data dir via cd -P (symlinks resolved) but compared it against the unresolved $HOME, spuriously failing the under-$HOME check. Fixed the test by pre-resolving HOME with cd -P — the exact pattern (and comment rationale) the sibling tilde tests in the same file already use; this first test simply predated them.

3. un-stamped DEFAULT_REF fails closed (install-bootstrap.bats) — host CLI leaking into the sandbox.
This is the only bootstrap test that sets no REF/BRANCH, so _tb_bail_ok stays armed — and run_boot's PATH="$BIN:$PATH" let the dev box's real /usr/local/bin/tracebloc satisfy command -v tracebloc. The test then ran the host's real tracebloc doctor and, the box being healthy, exec'd the real CLI, which exited 0 before the REF gate was ever reached (CI has no tracebloc binary, so it never saw this). Added run_boot_hermeticPATH=$BIN alone (setup already symlinks the needed coreutils for exactly this pattern) plus a sandboxed HOME, since the bootstrap re-prepends ~/.local/bin to PATH. Beyond determinism, this stops the suite from executing a real CLI binary on dev machines.

4. bats BW01 warning on sudo modprobe overlay (common.bats).
The sudo(): non-root without sudo returns 127 test deliberately expects 127, which bats flags as "command not found". Switched to run -127 plus the documented bats_require_minimum_version 1.5.0 declaration (CI's apt bats is 1.10, fine with both).

Related

Local dev-environment fix — no tracked issue.

Type of change

  • Bug fix
  • Tech-debt / refactor

Test plan

  • macOS (bash 3.2.57, bats 1.13.0): full suite bats scripts/tests/*.bats — 468/468 pass, zero warnings (was: 3 failures + BW01)
  • scripts/gen-manifest.sh --check — manifest up to date
  • CI-equivalent lint locally: bash -n over all scripts + shellcheck --severity=error on the same file set as the Lint job — clean
  • bash scripts/check-style.sh — clean
  • Behavior spot-check of the substitution fix on bash 3.2: 'a''b' unescapes to a'b; a'b escapes to a''b

Checklist

  • Tests added / updated and passing locally
  • Docs updated if behavior or config changed (N/A)
  • No secrets / credentials in the diff
  • For security-sensitive paths: appropriate reviewer requested (manifest regenerated; no gate/verify logic touched)
  • Terminal output follows STYLE.md — bash scripts/check-style.sh passes
  • Cross-repo issues use Fixes tracebloc/<repo>#N (N/A)

🤖 Generated with Claude Code


Note

Low Risk
Changes are installer script portability, values-file quoting hardening, and test isolation; manifest hash updated but no verify/bootstrap gate logic changed.

Overview
Makes the installer bats suite reliable on stock macOS (bash 3.2) while tightening how credentials land in generated Helm values.

Helm values / YAML: Adds shared _yaml_sq_escape / _yaml_sq_unescape so single-quoted YAML scalars work on bash 3.2 (escaped-quote replacements must use a variable, not \'). clientId is now written like clientPassword—single-quoted with escaping—instead of a double-quoted scalar that could break on " or \. scripts/manifest.sha256 is updated for the changed lib on the integrity surface.

Tests:validate_config resolves HOME with cd -P so macOS /var/private/var tmpdirs don’t fail the under-$HOME check. Bootstrap’s unstamped-ref test uses run_boot_hermetic so a host tracebloc CLI can’t bail out before the REF gate. run -127 for the sudo mock test plus bats_require_minimum_version 1.7.0. New bats cover YAML escape round-trips and clientId quoting.

Reviewed by Cursor Bugbot for commit 4f0872a. Bugbot is set up for automated code reviews on this repo. Configure here.

@LukasWodkaLukasWodka self-assigned this Jul 27, 2026
@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

bugbot run

@cursorcursorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 4e4c3bf. Configure here.

…tmpdir symlink + host-CLI leak)
Three independent reasons the suite failed on a stock macOS box but passed in CI:
- bash 3.2 keeps the backslash in a `\'` REPLACEMENT literal, so
"${TB_CLIENT_PASSWORD//\'/\'\'}" corrupts a quote-bearing password into
a\'\'b inside the generated values file. Replace via a $_sq variable,
which expands to a bare quote on 3.2 and 4/5 alike. (develop already
landed the same fix for _extract_yaml_value; this is the sibling call
site, which was still on the broken form.)
- macOS puts BATS_TEST_TMPDIR under the /var -> /private/var symlink while
validate_config resolves via `cd -P`, so the under-$HOME check failed
spuriously. Resolve $HOME with `cd -P` like the neighbouring tilde tests.
- the no-REF/BRANCH bootstrap tests exec'd the HOST's real `tracebloc`
CLI (on PATH or re-prepended ~/.local/bin) and exited 0 through the
already-installed bail-out, before reaching the gate under test. Add
run_boot_hermetic (sandboxed HOME + PATH) for those cases.
Also declares bats_require_minimum_version 1.5.0 for the `run -<code>`
syntax, and uses `run -127` where 127 is the asserted outcome (silences BW01).
Verified: 651/651 bats green on stock macOS bash 3.2 (GNU bash 3.2.57), and
shellcheck --severity=error + bash -n + check-style all clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@LukasWodka
LukasWodkaforce-pushed the fix/bats-macos-portability branch from 4e4c3bf to ac0d5cbCompareJuly 31, 2026 10:59
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
…ad-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>

@saqlainsyed007saqlainsyed007 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the escaping fix + test-portability changes. The core is sound and I verified the load-bearing claims locally: the bash 3.2 quote-corruption is real and the $_sq fix produces the correct a''b on 3.2.57; manifest.sha256 matches the new file and gen-manifest.sh --check passes; the validate_config HOME/cd -P fix and the hermetic-runner isolation both target the right root cause. No blocking bugs.

A few non-blocking observations inline — #1 (clientId) and #2 (bats version floor) are the two worth a look.

Comment threadscripts/lib/install-client-helm.sh Outdated
Comment threadscripts/tests/common.bats Outdated
Comment threadscripts/tests/common.bats Outdated
Comment threadscripts/lib/install-client-helm.sh Outdated
Comment threadscripts/tests/install-bootstrap.bats Outdated
…aqlain's review (#443)
All five review points, verified before acting on each.
1. clientId was interpolated RAW into a double-quoted scalar
(`clientId: "$TB_CLIENT_ID"`), so a `"` or `\` in the value would corrupt the
generated values file — the same bug class this PR fixes for the password one
line below, and unguarded (`_sanitize_credential` only strips paste and
non-printable characters). Both credentials now go through the shared escaper
into single-quoted scalars. verify_credentials gates IDs to UUIDs in practice,
so this is hardening, not a live break.
2. `bats_require_minimum_version 1.5.0` was self-defeating. Per bats-core's
changelog, `run -<N>` landed in 1.5.0 (2021-10-22) but
`bats_require_minimum_version` itself only exists from 1.7.0 (2022-05-14) — so
on a 1.5.x-1.6.x bats that line is an undefined command and kills the file
before the guard can help. Raised to the real floor, 1.7.0.
3. The `[ "$status" -eq 127 ]` after `run -127` was dead code. Confirmed
empirically: with a mismatched code bats fails AT the `run` line
("failed, expected exit code 127, got 3"), so nothing after it executes.
Dropped, with a comment saying why.
4. The bash-3.2 quote idiom now lives in exactly one place — `_yaml_sq_escape` /
`_yaml_sq_unescape`. Previously the escape and the unescape each carried their
own copy of the rationale and a cross-reference to keep them in sync; the rule
is now encoded once and both call sites just call it.
5. Tightened the `run_boot_hermetic` comment. The distinguishing condition is "no
REF/BRANCH *and* no mock tracebloc of its own" — the bail-out tests at ~226/242/254
also pass no REF/BRANCH but each writes its own mock into $BIN, so they are
hermetic by construction and correctly stay on plain `run_boot`.
Tests: 7 new — both escaper directions, a round-trip over quote-heavy values
("a'b", "'", "''", "it's a 'test'"), clientId surviving a single-quote round-trip,
a double quote no longer terminating the scalar early, and a guard that the
generated file uses the escaper rather than raw interpolation. Five existing
assertions on the GENERATED values file moved to the single-quoted form; the
double-quoted inputs and mocked `helm get values` output are deliberately left
as-is, since reading that form is the backward-compatibility path for values files
written by older installers.
Full bats suite 658/658 green on stock macOS bash 3.2 (GNU bash 3.2.57) —
including `validate_config: valid config passes`, which this PR fixes.
shellcheck --severity=error and --warning clean, bash -n, check-style,
check-drift and gen-manifest --check all clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

bugbot run

@cursorcursorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 4f0872a. Configure here.

@aptraceblocaptracebloc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving — verified all three fixes: bash-3.2 YAML escaping via a variable-built replacement (the \' literal keeps the backslash on macOS system bash), cd -P tmpdir-symlink resolution, and run_boot_hermetic isolating HOME/PATH so a host tracebloc CLI can't pass the test for the wrong reason. Plus the clientId single-quote hardening. manifest.sha256 updated; CI fully green.

@LukasWodka
LukasWodka merged commit d973ce8 into developJul 31, 2026
38 checks passed
@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

/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>
@LukasWodka
LukasWodka deleted the fix/bats-macos-portability branch August 14, 2026 13:53
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@LukasWodka@saqlainsyed007@aptracebloc