Uh oh!
There was an error while loading. Please reload this page.
fix(installer): make the macOS amd64-emulation gate engine-aware (client#748) - #756
Conversation
…ent#748) assert_amd64_emulation refused every Apple Silicon Mac whenever Rosetta amd64 was off, hard-erroring that 'the client images are amd64-only' — but only the MySQL 5.7 image is; the multi-arch 8.4 engine runs natively. A FRESH Mac that _mysql_engine_decision serves on 8.4 was turned away for emulation it does not need (Bugbot High). Ask the SAME rule the Linux gate uses: skip the smoke test when the engine resolves to 8.4. Safe before the cluster exists — the sticky check reads values_file and the datadir check reads HOST_DATA_DIR (default ~/.tracebloc), both known here, and on macOS the data lives on that host path, so an existing 5.7 install is still seen (helm-list existing_id, the one early-unavailable signal, is not how macOS surfaces prior data). Tighten the 5.7-path messages to name the 5.7 engine rather than 'all client images'. Tests: 8.4 -> skips the smoke / no docker run; 5.7 + broken emulation -> fails naming the 5.7 engine. Mutation-proved. The smoke-path tests now mock the engine to 5.7 explicitly rather than passing because the function is unsourced. 13/13 setup-macos-arch; manifest regenerated. Co-Authored-By: Claude Opus 5 <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 acbffcf. Configure here.
Uh oh!
There was an error while loading. Please reload this page.
… macOS gate Bugbot (High) on the first push: assert_amd64_emulation called _mysql_engine_decision directly, which reads TB_NAMESPACE via dynamic scope — and it is usually UNSET this early, so only HOST_DATA_DIR/mysql was probed, not the per-release HOST_DATA_DIR/<ns>/mysql. A re-run with existing 5.7 data in the per-release dir then resolved 8.4, skipped the smoke, and crash-looped later (no Linux gate on macOS). Call _pf_mysql_engine_decision instead — the same wrapper the preflight gate uses: it sets values_file and the SANITISED TB_NAMESPACE, and FAILS CLOSED to 5.7 if the engine lib is missing. 7/7 assert tests; manifest regenerated. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
aptracebloc
left a comment
There was a problem hiding this comment.
The core fix is right and the manifest regen is clean, but the change swaps a fail-closed gate for a fail-open one on the only arch gate macOS has, and the test that should hold down the Bugbot High it just fixed can't fail on it.
What it fixes, for the record. The old assert_amd64_emulation ran the Rosetta smoke test unconditionally on every arm64 Mac and hard-errored with "the client images are amd64-only". That went stale when MySQL 8.4 became multi-arch — only 5.7 is amd64-only now. So the false verdict was blocking a working configuration: a fresh Apple Silicon Mac that would have been served the native 8.4 engine was refused for emulation it never needed. Asking the engine decision and skipping the smoke on 8.4 is the correct shape.
1. scripts/lib/setup-macos.sh:694-698 — the skip trusts a verdict computed before helm is consulted
existing_id is a local of install_client_helm, which hasn't run at step b. So _mysql_engine_decision can't see a live release and falls through to 8.4 fresh. The wrapper's own comment (preflight.sh:369-372) acknowledges exactly this and says the residual case "is caught by _assert_engine_runs_on_this_arch" — but that backstop doesn't exist on macOS: install-client-helm.sh:961 is [[ "${OS:-}" == "Linux" ]] || return 0. That comment was written for Linux, where preflight is one of two gates. On macOS assert_amd64_emulation is now the only gate, and it has just opted out.
Two reachable paths, neither exotic:
TB_STORAGE_MODE=node-local— permitted on macOS (common.sh:823-827, validated at863-864, no Linux-only guard), and perinstall-client-helm.sh:901that is the documented case where both datadir probes come up empty.- A re-run whose
--data-dirdiffers from the first install (install-k8s.sh:165);HOST_DATA_DIRfalls back to~/.tracebloc(common.sh:845), so both the datadir probes and the stickyvalues.yamlcheck miss the real data.
Failure scenario: Apple Silicon Mac, Rosetta off, existing release. The gate prints "runs the client images natively", the install proceeds through cluster setup and credential entry, _resolve_mysql_engine then resolves 5.7 existing-release, _assert_engine_runs_on_this_arch returns early because OS != Linux, and the amd64-only MySQL 5.7 image crash-loops with no earlier signal. Pre-PR the always-run smoke test refused this correctly.
Cheapest fix is the late gate, not the early one: drop the OS == Linux early-return at install-client-helm.sh:961 so it covers macOS too. It already re-asks once the engine is real, so there's no need to make the early gate clairvoyant.
2. scripts/tests/setup-macos-arch.bats:30-31, 103-121 — the tests don't pin the fix they exist for
setup() stubs _mysql_engine_decision — the raw rule — plus _client_values_file, and leaves _client_default_namespace undefined, so the sanitised-TB_NAMESPACE branch at preflight.sh:400-402 never executes. Consequence: reverting line 694 from _pf_mysql_engine_decision back to _mysql_engine_decision — reintroducing the exact Bugbot High this PR fixes — leaves both new tests green. Under these mocks the wrapper and the raw rule are indistinguishable.
The body says "mutation-proved", and that's the one mutation that matters. preflight.bats:45-52 already has the right harness: _load_engine_rule sources the real install-client-helm.sh and _engine_ctx builds a real datadir on disk. Reusing it with a per-release HOST_DATA_DIR/<ns>/mysql fixture would pin it.
Non-blocking
- No undetermined-engine case for this gate.
setup()always defines_mysql_engine_decision, so the wrapper's fail-closed5.7 no-engine-rulepath never runs throughassert_amd64_emulation. The behaviour is correct and is covered for_pf_archatpreflight.bats:138, just not for the new macOS caller. awk '{print $1}'at:694discards the reason and adds a pipeline_pf_archdoesn't need — it uses_engine="${_decision%% *}"(preflight.sh:431-432). Dropping the reason means this gate can't tellinvalid <value>from a genuine 5.7, so a garbageTB_MYSQL_ENGINEon macOS passes the smoke test and is only refused much later in_resolve_mysql_engine; macOS_pf_archreturns early atpreflight.sh:418, so preflight never catches it either. Pre-existing on macOS, not a regression. The2>/dev/nullon the same line would mask a genuine stderr diagnostic, though there's nothing to mask while the wrapper always exits 0.
Verified clean
Manifest — recomputed, not trusted.setup-macos.sh at this head hashes to c5813ec583d80beebad0171491e21b95ef791c9873ad691de983b4ea111f5ffa, which is exactly line 7 of the committed scripts/manifest.sha256. Exactly one line changed and it's the right one. Also confirmed the base wasn't already stale (develop hashes to the removed c2cf1315…) and re-verified five other entries — install-k8s.sh, common.sh, preflight.sh, cluster.sh, install-client-helm.sh — against this ref. The installer won't reject its own file.
Shell safety.preflight.sh is sourced at install-k8s.sh:61, before setup-macos.sh at 65, so _pf_mysql_engine_decision is always defined — no 127-under-pipefail abort. Both decision functions return 0 on every path, so the bare assignment can't trip set -e. No SIGPIPE exposure: awk drains stdin, so this isn't the backend#1778 shape, and the rule itself already carries that fix at install-client-helm.sh:883. All new reads quoted; _engine declared local then assigned separately, which is why the status propagates at all. bash 3.2 clean.
No chart bump needed, established from the guard's logic rather than the file list: scripts/chart-version-guard.sh:82-83 classifies chart content as <chart>/templates/*, charts/*, crds/*, values.yaml or values.schema.json. scripts/** matches none.
— drafted with Claude Code
saadqbal
left a comment
There was a problem hiding this comment.
Second pass on this head — no new findings from me, but the standing blocker still stands, so re-flagging rather than approving over it.
Skipping the smoke on an 8.4 verdict is the right shape, and the 8.4-vs-5.7 split is correct for every case the early gate can actually see. The problem is the case it can't: existing_id isn't known until install_client_helm runs, so an existing release with no host datadir reads as 8.4 fresh here and later resolves to 5.7 existing-release. On macOS _assert_engine_runs_on_this_arch returns early (OS == Linux), so nothing re-asks — Apple Silicon + Rosetta off + existing release now proceeds to an exec-format crash-loop where the old unconditional smoke refused it. The --data-dir-changed and TB_STORAGE_MODE=node-local paths get there without anything exotic. Dropping the Linux early-return on the late gate is the cheaper fix than making this one clairvoyant.
I ran the mutation the review body claims is covered: swapping _pf_mysql_engine_decision back to the raw _mysql_engine_decision at setup-macos.sh:694 leaves all 13 tests green. setup() stubs the raw rule and never defines _client_default_namespace, so the wrapper's sanitising branch never runs and the two functions are indistinguishable under these mocks — the second commit's fix isn't pinned by anything.
Manifest checked by hand: setup-macos.sh at this head hashes to c5813ec5…, matching the committed line. 13/13 bats green at this head.
— drafted with Claude Code
…mulation check (Arturo, client#756)
Arturo's review: the early assert_amd64_emulation skips on an 8.4 GUESS, but
existing_id needs a live release (unknowable before helm), so on TB_STORAGE_MODE=
node-local or a --data-dir re-run an existing 5.7 release read as 8.4, skipped the
smoke, and crash-looped — the Linux backstop _assert_engine_runs_on_this_arch
returned early on macOS, so macOS had NO late gate.
- Extract _macos_amd64_emulation_ok (the Rosetta/Docker smoke, as a boolean) and
_macos_amd64_refusal (the Rosetta remedy). One emulation answer in one place.
- _assert_engine_runs_on_this_arch now runs on macOS too, with a PER-OS emulation
check (binfmt on Linux, the smoke on macOS — binfmt does not exist there) and
fails closed. The early gate stays optimistic; this is the backstop it relies on.
- assert_amd64_emulation uses the shared helpers + ${_decision%% *} (no awk, no
2>/dev/null), per _pf_arch.
Manifest regenerated. Tests follow in the next commit.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>…turo, client#756) Point 2 of the review: the macOS assert tests mocked _mysql_engine_decision (the raw rule), so the wrapper and the raw rule were indistinguishable — reverting the fix stayed green. Source the REAL rule (as preflight.bats does) and drive verdicts from HOST_DATA_DIR fixtures. New PER-RELEASE test builds HOST_DATA_DIR/<ns>/mysql 5.7 data with TB_NAMESPACE unset: the wrapper's sanitised namespace finds it and refuses; the raw rule misses it and skips — so reverting to the raw rule reddens this test (mutation-proved). Late gate gains macOS coverage: emulation-ok proceeds, emulation-missing and helper-absent both refuse (fail closed). 13/13 setup-macos-arch, 7/7 late gate. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
LukasWodka
commented
Aug 19, 2026
Both blocking points were real — thanks, this was a good catch. Addressed: 1. Fail-open → fail-closed backstop on macOS. You're right that the early 2. Tests pin the fix now. Dropped the Non-blocking: Two commits: the source (backstop + shared helpers) and the tests. Manifest regenerated. Ready for another look. |
aptracebloc
left a comment
There was a problem hiding this comment.
Re-reviewed the current head — my earlier change-request is addressed, so this approval supersedes it.
The fail-open gap is closed: _assert_engine_runs_on_this_arch now runs on macOS (Darwin branch, called after _resolve_mysql_engine) and refuses arm64 + 5.7 + emulation-missing — the fail-closed late backstop, exactly the "fix the late gate, don't make the early one clairvoyant" shape. And the test gap is closed: I ran the mutation — neutering the Darwin backstop now reddens the two new tests (macOS + 5.7 + emulation MISSING → refuses, helper ABSENT → refuses), where before the whole thing stayed green. Manifest re-signed correctly (shasum -c all OK), bats green.
Residual (non-blocking): the early-gate wrapper _pf_→raw mutation still leaves setup-macos-arch green, but that's no longer a safety concern now the late gate is the fail-closed net.
Code-owner @saadqbal's change-request still stands and is the remaining gate — over to him to clear now the backstop + pinning he asked for have landed.
LukasWodka
commented
Aug 19, 2026
/fr-pass Best-effort functional review passed (triage: merged clean, non-interactive; behavioral evidence limited while e2e journey is red — backend#2206). Advancing to Ready for prod. |

Resolves the Bugbot High on the client develop→staging mirror (#748),
scripts/lib/install-client-helm.sh:961→ the real cause isassert_amd64_emulationinsetup-macos.sh.Bug
On Apple Silicon,
_assert_engine_runs_on_this_archreturns early (Linux-only) andassert_amd64_emulationhard-errors that "the client images are amd64-only" whenever Rosetta amd64 is off — but only the MySQL 5.7 image is; the multi-arch 8.4 engine runs natively. A fresh Mac that_mysql_engine_decisionwould serve on 8.4 was refused for emulation it does not need. This is the macOS gap I deferred in #752; Bugbot rates it High, so it is fixed here.Fix
assert_amd64_emulationnow asks the same rule the Linux gate uses — skip the smoke test when the engine resolves to 8.4. Safe before the cluster exists: the sticky check readsvalues_fileand the datadir check readsHOST_DATA_DIR(default~/.tracebloc), both known here, and on macOS the data lives on that host path — so an existing 5.7 install is still detected (helm listexisting_id, the one signal unavailable this early, is not how macOS surfaces prior data, so no fail-open). The 5.7-path messages now name the 5.7 engine rather than "all client images".Tests
engine → 8.4skips the smoke, nodocker run, not refused.engine 5.7 + broken emulationfails, naming the 5.7 engine.setup-macos-arch; manifest regenerated.Addresses @saadqbal’s original datadir thread too — that concern (existing_id →
existing-datadir) was already split out in #752 (existing-release); replying to resolve it separately.On
developfor the staging-hop re-prepare.Note
Medium Risk
Changes early-install gating on Apple Silicon where wrong engine detection could either block fresh 8.4 installs or allow 5.7 without emulation; mitigated by fail-closed wrapper and new bats tests.
Overview
Apple Silicon installs no longer fail the amd64 emulation smoke test when MySQL resolves to the multi-arch 8.4 engine — only the MySQL 5.7 image still requires Rosetta.
assert_amd64_emulationnow calls_pf_mysql_engine_decision(same wrapper as Linux preflight) so per-release datadir and values-file context are included; when the engine is 8.4 it skipsdocker run --platform linux/amd64and succeeds. When the install stays on 5.7, behavior is unchanged but warnings/errors name the 5.7 engine instead of implying all client images are amd64-only.Bats coverage adds the 8.4 skip path and 5.7 failure messaging; default mocks pin the engine to 5.7 so existing smoke tests stay deterministic.
scripts/manifest.sha256is updated forsetup-macos.sh.Reviewed by Cursor Bugbot for commit 9ad98e9. Bugbot is set up for automated code reviews on this repo. Configure here.